[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63082/new/

https://reviews.llvm.org/D63082



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8515
   "%0 is not supported on this target">;
+def err_type_not_equivalent : Error<
+  "%0 is not equivalent between host and target">;

No, this message shall tell something like `host requires %0 bit size %1 type 
support, but device %3 does not support it`. Here `%0` is the sizr of the host 
type in bits, `%1` is the type name and `%2` is the device triple.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

Why do we need all this stuff? I think the original code works good here, we 
just need to improve the message.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

ABataev wrote:
> Why do we need all this stuff? I think the original code works good here, we 
> just need to improve the message.
> Why do we need all this stuff? I think the original code works good here, we 
> just need to improve the message.

It seems we've been miscommunicating.  Let's review the discussion so far, 
point by point, and I'll show you how I arrived at this code.  I think the 
first major point is as follows.

I asked:
> Are you intentionally requiring support for __float128 when the source type 
> is 128-bit long double? That seems to mean powerpc64le cannot offload to 
> itself.

You replied:
> No, if the host has 128 bit long double, the device must also have 128 bit 
> long double. It has nothing to do with the float128 type itself.

I thought you were agreeing with my understanding.  That is, the original code 
requires `__float128` support even when 128-bit `long double` is in use.  
That's why powerpc64le cannot offload to itself.  How does the original code 
require `__float128`?  It checks `Context.getTargetInfo().hasFloat128Type()`.  
As far as I can tell, that checks for `__float128` support, and it does not 
check for 128-bit `long double` support.  That's why powerpc64le cannot offload 
to itself.

I'll review other points later so we can discuss them.  First, let's see if we 
can agree on this point.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

jdenny wrote:
> ABataev wrote:
> > Why do we need all this stuff? I think the original code works good here, 
> > we just need to improve the message.
> > Why do we need all this stuff? I think the original code works good here, 
> > we just need to improve the message.
> 
> It seems we've been miscommunicating.  Let's review the discussion so far, 
> point by point, and I'll show you how I arrived at this code.  I think the 
> first major point is as follows.
> 
> I asked:
> > Are you intentionally requiring support for __float128 when the source type 
> > is 128-bit long double? That seems to mean powerpc64le cannot offload to 
> > itself.
> 
> You replied:
> > No, if the host has 128 bit long double, the device must also have 128 bit 
> > long double. It has nothing to do with the float128 type itself.
> 
> I thought you were agreeing with my understanding.  That is, the original 
> code requires `__float128` support even when 128-bit `long double` is in use. 
>  That's why powerpc64le cannot offload to itself.  How does the original code 
> require `__float128`?  It checks `Context.getTargetInfo().hasFloat128Type()`. 
>  As far as I can tell, that checks for `__float128` support, and it does not 
> check for 128-bit `long double` support.  That's why powerpc64le cannot 
> offload to itself.
> 
> I'll review other points later so we can discuss them.  First, let's see if 
> we can agree on this point.
> 
My point about ppc64le is:
If the host code uses long double 128 bit long, tbe device long double also 
must be 128 bit long. But if device does not support 128bit FP type naturally, 
user cannot do any operations with it except just load/stores.
Forget about float128 type, we talk about 128bit long double here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

ABataev wrote:
> jdenny wrote:
> > ABataev wrote:
> > > Why do we need all this stuff? I think the original code works good here, 
> > > we just need to improve the message.
> > > Why do we need all this stuff? I think the original code works good here, 
> > > we just need to improve the message.
> > 
> > It seems we've been miscommunicating.  Let's review the discussion so far, 
> > point by point, and I'll show you how I arrived at this code.  I think the 
> > first major point is as follows.
> > 
> > I asked:
> > > Are you intentionally requiring support for __float128 when the source 
> > > type is 128-bit long double? That seems to mean powerpc64le cannot 
> > > offload to itself.
> > 
> > You replied:
> > > No, if the host has 128 bit long double, the device must also have 128 
> > > bit long double. It has nothing to do with the float128 type itself.
> > 
> > I thought you were agreeing with my understanding.  That is, the original 
> > code requires `__float128` support even when 128-bit `long double` is in 
> > use.  That's why powerpc64le cannot offload to itself.  How does the 
> > original code require `__float128`?  It checks 
> > `Context.getTargetInfo().hasFloat128Type()`.  As far as I can tell, that 
> > checks for `__float128` support, and it does not check for 128-bit `long 
> > double` support.  That's why powerpc64le cannot offload to itself.
> > 
> > I'll review other points later so we can discuss them.  First, let's see if 
> > we can agree on this point.
> > 
> My point about ppc64le is:
> If the host code uses long double 128 bit long, tbe device long double also 
> must be 128 bit long. But if device does not support 128bit FP type 
> naturally, user cannot do any operations with it except just load/stores.
> Forget about float128 type, we talk about 128bit long double here.
This patch adds new testing for powerpc64le.  Without the rest of this patch 
(and without the `expected-error` change), should that pass?  It does not for 
me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

jdenny wrote:
> ABataev wrote:
> > jdenny wrote:
> > > ABataev wrote:
> > > > Why do we need all this stuff? I think the original code works good 
> > > > here, we just need to improve the message.
> > > > Why do we need all this stuff? I think the original code works good 
> > > > here, we just need to improve the message.
> > > 
> > > It seems we've been miscommunicating.  Let's review the discussion so 
> > > far, point by point, and I'll show you how I arrived at this code.  I 
> > > think the first major point is as follows.
> > > 
> > > I asked:
> > > > Are you intentionally requiring support for __float128 when the source 
> > > > type is 128-bit long double? That seems to mean powerpc64le cannot 
> > > > offload to itself.
> > > 
> > > You replied:
> > > > No, if the host has 128 bit long double, the device must also have 128 
> > > > bit long double. It has nothing to do with the float128 type itself.
> > > 
> > > I thought you were agreeing with my understanding.  That is, the original 
> > > code requires `__float128` support even when 128-bit `long double` is in 
> > > use.  That's why powerpc64le cannot offload to itself.  How does the 
> > > original code require `__float128`?  It checks 
> > > `Context.getTargetInfo().hasFloat128Type()`.  As far as I can tell, that 
> > > checks for `__float128` support, and it does not check for 128-bit `long 
> > > double` support.  That's why powerpc64le cannot offload to itself.
> > > 
> > > I'll review other points later so we can discuss them.  First, let's see 
> > > if we can agree on this point.
> > > 
> > My point about ppc64le is:
> > If the host code uses long double 128 bit long, tbe device long double also 
> > must be 128 bit long. But if device does not support 128bit FP type 
> > naturally, user cannot do any operations with it except just load/stores.
> > Forget about float128 type, we talk about 128bit long double here.
> This patch adds new testing for powerpc64le.  Without the rest of this patch 
> (and without the `expected-error` change), should that pass?  It does not for 
> me.
Just like I said before, it means that your device config does not match the 
expected one. It means that either you specified incorrect triple, or the 
device is not configured properly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

ABataev wrote:
> jdenny wrote:
> > ABataev wrote:
> > > jdenny wrote:
> > > > ABataev wrote:
> > > > > Why do we need all this stuff? I think the original code works good 
> > > > > here, we just need to improve the message.
> > > > > Why do we need all this stuff? I think the original code works good 
> > > > > here, we just need to improve the message.
> > > > 
> > > > It seems we've been miscommunicating.  Let's review the discussion so 
> > > > far, point by point, and I'll show you how I arrived at this code.  I 
> > > > think the first major point is as follows.
> > > > 
> > > > I asked:
> > > > > Are you intentionally requiring support for __float128 when the 
> > > > > source type is 128-bit long double? That seems to mean powerpc64le 
> > > > > cannot offload to itself.
> > > > 
> > > > You replied:
> > > > > No, if the host has 128 bit long double, the device must also have 
> > > > > 128 bit long double. It has nothing to do with the float128 type 
> > > > > itself.
> > > > 
> > > > I thought you were agreeing with my understanding.  That is, the 
> > > > original code requires `__float128` support even when 128-bit `long 
> > > > double` is in use.  That's why powerpc64le cannot offload to itself.  
> > > > How does the original code require `__float128`?  It checks 
> > > > `Context.getTargetInfo().hasFloat128Type()`.  As far as I can tell, 
> > > > that checks for `__float128` support, and it does not check for 128-bit 
> > > > `long double` support.  That's why powerpc64le cannot offload to itself.
> > > > 
> > > > I'll review other points later so we can discuss them.  First, let's 
> > > > see if we can agree on this point.
> > > > 
> > > My point about ppc64le is:
> > > If the host code uses long double 128 bit long, tbe device long double 
> > > also must be 128 bit long. But if device does not support 128bit FP type 
> > > naturally, user cannot do any operations with it except just load/stores.
> > > Forget about float128 type, we talk about 128bit long double here.
> > This patch adds new testing for powerpc64le.  Without the rest of this 
> > patch (and without the `expected-error` change), should that pass?  It does 
> > not for me.
> Just like I said before, it means that your device config does not match the 
> expected one. It means that either you specified incorrect triple, or the 
> device is not configured properly.
The triple is in the new test code.  Is it incorrect?

When you discussed device configuration earlier, you made a comment about 64 
bit FP, and I couldn't find evidence of that anywhere.

When I explore the device config as you suggested earlier, as far as I can 
tell, the only issue here is that it claims powerpc64le does not support 
`__float128`, but the original logic in `Sema::checkOpenMPDeviceExpr` requires 
`__float128` by checking `Context.getTargetInfo().hasFloat128Type()`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

jdenny wrote:
> ABataev wrote:
> > jdenny wrote:
> > > ABataev wrote:
> > > > jdenny wrote:
> > > > > ABataev wrote:
> > > > > > Why do we need all this stuff? I think the original code works good 
> > > > > > here, we just need to improve the message.
> > > > > > Why do we need all this stuff? I think the original code works good 
> > > > > > here, we just need to improve the message.
> > > > > 
> > > > > It seems we've been miscommunicating.  Let's review the discussion so 
> > > > > far, point by point, and I'll show you how I arrived at this code.  I 
> > > > > think the first major point is as follows.
> > > > > 
> > > > > I asked:
> > > > > > Are you intentionally requiring support for __float128 when the 
> > > > > > source type is 128-bit long double? That seems to mean powerpc64le 
> > > > > > cannot offload to itself.
> > > > > 
> > > > > You replied:
> > > > > > No, if the host has 128 bit long double, the device must also have 
> > > > > > 128 bit long double. It has nothing to do with the float128 type 
> > > > > > itself.
> > > > > 
> > > > > I thought you were agreeing with my understanding.  That is, the 
> > > > > original code requires `__float128` support even when 128-bit `long 
> > > > > double` is in use.  That's why powerpc64le cannot offload to itself.  
> > > > > How does the original code require `__float128`?  It checks 
> > > > > `Context.getTargetInfo().hasFloat128Type()`.  As far as I can tell, 
> > > > > that checks for `__float128` support, and it does not check for 
> > > > > 128-bit `long double` support.  That's why powerpc64le cannot offload 
> > > > > to itself.
> > > > > 
> > > > > I'll review other points later so we can discuss them.  First, let's 
> > > > > see if we can agree on this point.
> > > > > 
> > > > My point about ppc64le is:
> > > > If the host code uses long double 128 bit long, tbe device long double 
> > > > also must be 128 bit long. But if device does not support 128bit FP 
> > > > type naturally, user cannot do any operations with it except just 
> > > > load/stores.
> > > > Forget about float128 type, we talk about 128bit long double here.
> > > This patch adds new testing for powerpc64le.  Without the rest of this 
> > > patch (and without the `expected-error` change), should that pass?  It 
> > > does not for me.
> > Just like I said before, it means that your device config does not match 
> > the expected one. It means that either you specified incorrect triple, or 
> > the device is not configured properly.
> The triple is in the new test code.  Is it incorrect?
> 
> When you discussed device configuration earlier, you made a comment about 64 
> bit FP, and I couldn't find evidence of that anywhere.
> 
> When I explore the device config as you suggested earlier, as far as I can 
> tell, the only issue here is that it claims powerpc64le does not support 
> `__float128`, but the original logic in `Sema::checkOpenMPDeviceExpr` 
> requires `__float128` by checking `Context.getTargetInfo().hasFloat128Type()`.
Original ppc64le host reports that it supports 128bit float. The device reports 
that it does not support it. It just means that the device has different 
configuration than host. If yoh want the device to support 128 bit FP as the 
host, you need either to specify the device correctly, orfix the target device 
configuration in the lib/Basic/Target if it does not match the expected 
behavior. 
Check the host init for ppc64le. It sets hasFloat128 to true. For some reason, 
device target config does not set this flag and reports that it does not 
support 128bit FP. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

ABataev wrote:
> jdenny wrote:
> > ABataev wrote:
> > > jdenny wrote:
> > > > ABataev wrote:
> > > > > jdenny wrote:
> > > > > > ABataev wrote:
> > > > > > > Why do we need all this stuff? I think the original code works 
> > > > > > > good here, we just need to improve the message.
> > > > > > > Why do we need all this stuff? I think the original code works 
> > > > > > > good here, we just need to improve the message.
> > > > > > 
> > > > > > It seems we've been miscommunicating.  Let's review the discussion 
> > > > > > so far, point by point, and I'll show you how I arrived at this 
> > > > > > code.  I think the first major point is as follows.
> > > > > > 
> > > > > > I asked:
> > > > > > > Are you intentionally requiring support for __float128 when the 
> > > > > > > source type is 128-bit long double? That seems to mean 
> > > > > > > powerpc64le cannot offload to itself.
> > > > > > 
> > > > > > You replied:
> > > > > > > No, if the host has 128 bit long double, the device must also 
> > > > > > > have 128 bit long double. It has nothing to do with the float128 
> > > > > > > type itself.
> > > > > > 
> > > > > > I thought you were agreeing with my understanding.  That is, the 
> > > > > > original code requires `__float128` support even when 128-bit `long 
> > > > > > double` is in use.  That's why powerpc64le cannot offload to 
> > > > > > itself.  How does the original code require `__float128`?  It 
> > > > > > checks `Context.getTargetInfo().hasFloat128Type()`.  As far as I 
> > > > > > can tell, that checks for `__float128` support, and it does not 
> > > > > > check for 128-bit `long double` support.  That's why powerpc64le 
> > > > > > cannot offload to itself.
> > > > > > 
> > > > > > I'll review other points later so we can discuss them.  First, 
> > > > > > let's see if we can agree on this point.
> > > > > > 
> > > > > My point about ppc64le is:
> > > > > If the host code uses long double 128 bit long, tbe device long 
> > > > > double also must be 128 bit long. But if device does not support 
> > > > > 128bit FP type naturally, user cannot do any operations with it 
> > > > > except just load/stores.
> > > > > Forget about float128 type, we talk about 128bit long double here.
> > > > This patch adds new testing for powerpc64le.  Without the rest of this 
> > > > patch (and without the `expected-error` change), should that pass?  It 
> > > > does not for me.
> > > Just like I said before, it means that your device config does not match 
> > > the expected one. It means that either you specified incorrect triple, or 
> > > the device is not configured properly.
> > The triple is in the new test code.  Is it incorrect?
> > 
> > When you discussed device configuration earlier, you made a comment about 
> > 64 bit FP, and I couldn't find evidence of that anywhere.
> > 
> > When I explore the device config as you suggested earlier, as far as I can 
> > tell, the only issue here is that it claims powerpc64le does not support 
> > `__float128`, but the original logic in `Sema::checkOpenMPDeviceExpr` 
> > requires `__float128` by checking 
> > `Context.getTargetInfo().hasFloat128Type()`.
> Original ppc64le host reports that it supports 128bit float. The device 
> reports that it does not support it. It just means that the device has 
> different configuration than host. If yoh want the device to support 128 bit 
> FP as the host, you need either to specify the device correctly, orfix the 
> target device configuration in the lib/Basic/Target if it does not match the 
> expected behavior. 
> Check the host init for ppc64le. It sets hasFloat128 to true. For some 
> reason, device target config does not set this flag and reports that it does 
> not support 128bit FP. 
> Original ppc64le host reports that it supports 128bit float.

> Check the host init for ppc64le. It sets hasFloat128 to true.

Doesn't appear to happen for me unless I add `-target-feature +float128` as a 
`-cc1` option, as some existing powerpc64le clang tests do.

@MaskRay, will powerpc64le set `HasFloat128=true` after your changes?



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61467: [Rewrite] Extend to further accept CharSourceRange

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: cfe/trunk/unittests/Rewrite/CMakeLists.txt:12
   clangRewrite
+  clangTooling
   )

This makes RewriteTests depend on clangTooling, and in follow-ups on 
clangFrontend and clangSerialization. Rewrite used to depend on basically only 
clangBasic and clangLex, and now it depends on almost all of clang. Maybe 
there's a less heavy-weight way to test this?

Also, when do you expect to land code that uses this? Checking in code that's 
unused upstream over a long period of time seems suboptimal. I delete code that 
shows up unused on http://llvm-cs.pcc.me.uk/ every now and then for example. 
(Ignore this part if you expect to check in clients of the overload soon.)


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61467/new/

https://reviews.llvm.org/D61467



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

jdenny wrote:
> ABataev wrote:
> > jdenny wrote:
> > > ABataev wrote:
> > > > jdenny wrote:
> > > > > ABataev wrote:
> > > > > > jdenny wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Why do we need all this stuff? I think the original code works 
> > > > > > > > good here, we just need to improve the message.
> > > > > > > > Why do we need all this stuff? I think the original code works 
> > > > > > > > good here, we just need to improve the message.
> > > > > > > 
> > > > > > > It seems we've been miscommunicating.  Let's review the 
> > > > > > > discussion so far, point by point, and I'll show you how I 
> > > > > > > arrived at this code.  I think the first major point is as 
> > > > > > > follows.
> > > > > > > 
> > > > > > > I asked:
> > > > > > > > Are you intentionally requiring support for __float128 when the 
> > > > > > > > source type is 128-bit long double? That seems to mean 
> > > > > > > > powerpc64le cannot offload to itself.
> > > > > > > 
> > > > > > > You replied:
> > > > > > > > No, if the host has 128 bit long double, the device must also 
> > > > > > > > have 128 bit long double. It has nothing to do with the 
> > > > > > > > float128 type itself.
> > > > > > > 
> > > > > > > I thought you were agreeing with my understanding.  That is, the 
> > > > > > > original code requires `__float128` support even when 128-bit 
> > > > > > > `long double` is in use.  That's why powerpc64le cannot offload 
> > > > > > > to itself.  How does the original code require `__float128`?  It 
> > > > > > > checks `Context.getTargetInfo().hasFloat128Type()`.  As far as I 
> > > > > > > can tell, that checks for `__float128` support, and it does not 
> > > > > > > check for 128-bit `long double` support.  That's why powerpc64le 
> > > > > > > cannot offload to itself.
> > > > > > > 
> > > > > > > I'll review other points later so we can discuss them.  First, 
> > > > > > > let's see if we can agree on this point.
> > > > > > > 
> > > > > > My point about ppc64le is:
> > > > > > If the host code uses long double 128 bit long, tbe device long 
> > > > > > double also must be 128 bit long. But if device does not support 
> > > > > > 128bit FP type naturally, user cannot do any operations with it 
> > > > > > except just load/stores.
> > > > > > Forget about float128 type, we talk about 128bit long double here.
> > > > > This patch adds new testing for powerpc64le.  Without the rest of 
> > > > > this patch (and without the `expected-error` change), should that 
> > > > > pass?  It does not for me.
> > > > Just like I said before, it means that your device config does not 
> > > > match the expected one. It means that either you specified incorrect 
> > > > triple, or the device is not configured properly.
> > > The triple is in the new test code.  Is it incorrect?
> > > 
> > > When you discussed device configuration earlier, you made a comment about 
> > > 64 bit FP, and I couldn't find evidence of that anywhere.
> > > 
> > > When I explore the device config as you suggested earlier, as far as I 
> > > can tell, the only issue here is that it claims powerpc64le does not 
> > > support `__float128`, but the original logic in 
> > > `Sema::checkOpenMPDeviceExpr` requires `__float128` by checking 
> > > `Context.getTargetInfo().hasFloat128Type()`.
> > Original ppc64le host reports that it supports 128bit float. The device 
> > reports that it does not support it. It just means that the device has 
> > different configuration than host. If yoh want the device to support 128 
> > bit FP as the host, you need either to specify the device correctly, orfix 
> > the target device configuration in the lib/Basic/Target if it does not 
> > match the expected behavior. 
> > Check the host init for ppc64le. It sets hasFloat128 to true. For some 
> > reason, device target config does not set this flag and reports that it 
> > does not support 128bit FP. 
> > Original ppc64le host reports that it supports 128bit float.
> 
> > Check the host init for ppc64le. It sets hasFloat128 to true.
> 
> Doesn't appear to happen for me unless I add `-target-feature +float128` as a 
> `-cc1` option, as some existing powerpc64le clang tests do.
> 
> @MaskRay, will powerpc64le set `HasFloat128=true` after your changes?
> 
This is what I meant! We just missed some special target config options for the 
device. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64289/new/

https://reviews.llvm.org/D64289



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61467: [Rewrite] Extend to further accept CharSourceRange

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: cfe/trunk/unittests/Rewrite/CMakeLists.txt:12
   clangRewrite
+  clangTooling
   )

thakis wrote:
> This makes RewriteTests depend on clangTooling, and in follow-ups on 
> clangFrontend and clangSerialization. Rewrite used to depend on basically 
> only clangBasic and clangLex, and now it depends on almost all of clang. 
> Maybe there's a less heavy-weight way to test this?
> 
> Also, when do you expect to land code that uses this? Checking in code that's 
> unused upstream over a long period of time seems suboptimal. I delete code 
> that shows up unused on http://llvm-cs.pcc.me.uk/ every now and then for 
> example. (Ignore this part if you expect to check in clients of the overload 
> soon.)
> This makes RewriteTests depend on clangTooling, and in follow-ups on 
> clangFrontend and clangSerialization. Rewrite used to depend on basically 
> only clangBasic and clangLex, and now it depends on almost all of clang. 
> Maybe there's a less heavy-weight way to test this?

Sure, I can look for another way to test this.  I didn't realize this would be 
a real problem.  But

> Also, when do you expect to land code that uses this?

Not soon.  I offered this patch thinking that obvious Rewrite extensions like 
this one could prove useful to the larger community even if there are no 
upstream users now.  I'm also fine to revert and keep my Rewrite extensions 
private for now.  Is that the best approach?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61467/new/

https://reviews.llvm.org/D61467



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Joel E. Denny via Phabricator via cfe-commits]

jdenny abandoned this revision.
jdenny added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

ABataev wrote:
> jdenny wrote:
> > ABataev wrote:
> > > jdenny wrote:
> > > > ABataev wrote:
> > > > > jdenny wrote:
> > > > > > ABataev wrote:
> > > > > > > jdenny wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > Why do we need all this stuff? I think the original code 
> > > > > > > > > works good here, we just need to improve the message.
> > > > > > > > > Why do we need all this stuff? I think the original code 
> > > > > > > > > works good here, we just need to improve the message.
> > > > > > > > 
> > > > > > > > It seems we've been miscommunicating.  Let's review the 
> > > > > > > > discussion so far, point by point, and I'll show you how I 
> > > > > > > > arrived at this code.  I think the first major point is as 
> > > > > > > > follows.
> > > > > > > > 
> > > > > > > > I asked:
> > > > > > > > > Are you intentionally requiring support for __float128 when 
> > > > > > > > > the source type is 128-bit long double? That seems to mean 
> > > > > > > > > powerpc64le cannot offload to itself.
> > > > > > > > 
> > > > > > > > You replied:
> > > > > > > > > No, if the host has 128 bit long double, the device must also 
> > > > > > > > > have 128 bit long double. It has nothing to do with the 
> > > > > > > > > float128 type itself.
> > > > > > > > 
> > > > > > > > I thought you were agreeing with my understanding.  That is, 
> > > > > > > > the original code requires `__float128` support even when 
> > > > > > > > 128-bit `long double` is in use.  That's why powerpc64le cannot 
> > > > > > > > offload to itself.  How does the original code require 
> > > > > > > > `__float128`?  It checks 
> > > > > > > > `Context.getTargetInfo().hasFloat128Type()`.  As far as I can 
> > > > > > > > tell, that checks for `__float128` support, and it does not 
> > > > > > > > check for 128-bit `long double` support.  That's why 
> > > > > > > > powerpc64le cannot offload to itself.
> > > > > > > > 
> > > > > > > > I'll review other points later so we can discuss them.  First, 
> > > > > > > > let's see if we can agree on this point.
> > > > > > > > 
> > > > > > > My point about ppc64le is:
> > > > > > > If the host code uses long double 128 bit long, tbe device long 
> > > > > > > double also must be 128 bit long. But if device does not support 
> > > > > > > 128bit FP type naturally, user cannot do any operations with it 
> > > > > > > except just load/stores.
> > > > > > > Forget about float128 type, we talk about 128bit long double here.
> > > > > > This patch adds new testing for powerpc64le.  Without the rest of 
> > > > > > this patch (and without the `expected-error` change), should that 
> > > > > > pass?  It does not for me.
> > > > > Just like I said before, it means that your device config does not 
> > > > > match the expected one. It means that either you specified incorrect 
> > > > > triple, or the device is not configured properly.
> > > > The triple is in the new test code.  Is it incorrect?
> > > > 
> > > > When you discussed device configuration earlier, you made a comment 
> > > > about 64 bit FP, and I couldn't find evidence of that anywhere.
> > > > 
> > > > When I explore the device config as you suggested earlier, as far as I 
> > > > can tell, the only issue here is that it claims powerpc64le does not 
> > > > support `__float128`, but the original logic in 
> > > > `Sema::checkOpenMPDeviceExpr` requires `__float128` by checking 
> > > > `Context.getTargetInfo().hasFloat128Type()`.
> > > Original ppc64le host reports that it supports 128bit float. The device 
> > > reports that it does not support it. It just means that the device has 
> > > different configuration than host. If yoh want the device to support 128 
> > > bit FP as the host, you need either to specify the device correctly, 
> > > orfix the target device configuration in the lib/Basic/Target if it does 
> > > not match the expected behavior. 
> > > Check the host init for ppc64le. It sets hasFloat128 to true. For some 
> > > reason, device target config does not set this flag and reports that it 
> > > does not support 128bit FP. 
> > > Original ppc64le host reports that it supports 128bit float.
> > 
> > > Check the host init for ppc64le. It sets hasFloat128 to true.
> > 
> > Doesn't appear to happen for me unless I add `-target-feature +float128` as 
> > a `-cc1` option, as some existing powerpc64le clang tests do.
> > 
> > @MaskRay, will powerpc64le set `HasFloat128=true` after your changes?
> > 
> This is what I meant! We just missed some special target config options for 
> the device. 
Thanks for the discussion.  I know what to do for the x86_64 case.  Others seem 
to be working on powerpc64le.  Maybe all architectures we care ab

[PATCH] D64276: [ItaniumMangle] Replace useFloat128ManglingForLongDouble() with getManglingForLongDouble() and getManglingForFloat128()

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 208294.
MaskRay edited the summary of this revision.
MaskRay removed a subscriber: wuzish.
MaskRay added a comment.

"U10__float128" -> "u9__ieee128"

This is the new mangled type in gcc>8.1


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64276/new/

https://reviews.llvm.org/D64276

Files:
  include/clang/Basic/TargetInfo.h
  lib/AST/ItaniumMangle.cpp
  lib/Basic/Targets/PPC.h
  lib/Basic/Targets/SystemZ.h
  lib/Basic/Targets/X86.h
  test/CodeGenCXX/float128-declarations.cpp

Index: test/CodeGenCXX/float128-declarations.cpp
===
--- test/CodeGenCXX/float128-declarations.cpp
+++ test/CodeGenCXX/float128-declarations.cpp
@@ -84,15 +84,15 @@
 // CHECK-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global fp128 0xL40040800
 // CHECK-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x fp128]
 // CHECK-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x fp128] [fp128 0xL3FFF, fp128 0xL40008000, fp128 0xL4025176592E0]
-// CHECK-DAG: define internal fp128 @_ZN12_GLOBAL__N_16func1nERKU10__float128(fp128*
+// CHECK-DAG: define internal fp128 @_ZN12_GLOBAL__N_16func1nERKu9__ieee128(fp128*
 // CHECK-DAG: @f1f = global fp128 0xL
 // CHECK-DAG: @f2f = global fp128 0xL40040333
 // CHECK-DAG: @arr1f = global [10 x fp128]
 // CHECK-DAG: @arr2f = global [3 x fp128] [fp128 0xLBFFF, fp128 0xLC0008000, fp128 0xLC025176592E0]
-// CHECK-DAG: declare fp128 @_Z6func1fU10__float128(fp128)
-// CHECK-DAG: define linkonce_odr void @_ZN2C1C2EU10__float128(%class.C1* %this, fp128 %arg)
-// CHECK-DAG: define linkonce_odr fp128 @_ZN2C16func2cEU10__float128(fp128 %arg)
-// CHECK-DAG: define linkonce_odr fp128 @_Z6func1tIU10__float128ET_S0_(fp128 %arg)
+// CHECK-DAG: declare fp128 @_Z6func1fu9__ieee128(fp128)
+// CHECK-DAG: define linkonce_odr void @_ZN2C1C2Eu9__ieee128(%class.C1* %this, fp128 %arg)
+// CHECK-DAG: define linkonce_odr fp128 @_ZN2C16func2cEu9__ieee128(fp128 %arg)
+// CHECK-DAG: define linkonce_odr fp128 @_Z6func1tIu9__ieee128ET_S0_(fp128 %arg)
 // CHECK-DAG: @__const.main.s1 = private unnamed_addr constant %struct.S1 { fp128 0xL40060800 }
 // CHECK-DAG: store fp128 0xLF0AFD0EBFF292DCE42E0B38CDD83F26F, fp128* %f1l, align 16
 // CHECK-DAG: store fp128 0xL8000, fp128* %f2l, align 16
Index: lib/Basic/Targets/X86.h
===
--- lib/Basic/Targets/X86.h
+++ lib/Basic/Targets/X86.h
@@ -848,7 +848,7 @@
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
   }
 
-  bool useFloat128ManglingForLongDouble() const override { return true; }
+  const char *getManglingForLongDouble() const override { return "g"; }
 };
 } // namespace targets
 } // namespace clang
Index: lib/Basic/Targets/SystemZ.h
===
--- lib/Basic/Targets/SystemZ.h
+++ lib/Basic/Targets/SystemZ.h
@@ -141,7 +141,7 @@
 return "";
   }
 
-  bool useFloat128ManglingForLongDouble() const override { return true; }
+  const char *getManglingForLongDouble() const override { return "g"; }
 };
 } // namespace targets
 } // namespace clang
Index: lib/Basic/Targets/PPC.h
===
--- lib/Basic/Targets/PPC.h
+++ lib/Basic/Targets/PPC.h
@@ -314,10 +314,15 @@
 
   bool hasSjLjLowering() const override { return true; }
 
-  bool useFloat128ManglingForLongDouble() const override {
-return LongDoubleWidth == 128 &&
-   LongDoubleFormat == &llvm::APFloat::PPCDoubleDouble() &&
-   getTriple().isOSBinFormatELF();
+  const char *getManglingForLongDouble() const override {
+if (LongDoubleWidth == 64)
+  return "e";
+return LongDoubleFormat == &llvm::APFloat::PPCDoubleDouble()
+   ? "g"
+   : "u9__ieee128";
+  }
+  const char *getManglingForFloat128() const override {
+return "u9__ieee128";
   }
 };
 
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -2608,30 +2608,19 @@
 Out << 'd';
 break;
   case BuiltinType::LongDouble: {
-bool UseFloat128Mangling =
-getASTContext().getTargetInfo().useFloat128ManglingForLongDouble();
-if (getASTContext().getLangOpts().OpenMP &&
-getASTContext().getLangOpts().OpenMPIsDevice) {
-  UseFloat128Mangling = getASTContext()
-.getAuxTargetInfo()
-->useFloat128ManglingForLongDouble();
-}
-Out << (UseFloat128Mangling ? 'g' : 'e');
+const TargetInfo *TI = getASTContext().getLangOpts().OpenMP &&
+

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

jdenny wrote:
> ABataev wrote:
> > jdenny wrote:
> > > ABataev wrote:
> > > > jdenny wrote:
> > > > > ABataev wrote:
> > > > > > jdenny wrote:
> > > > > > > ABataev wrote:
> > > > > > > > jdenny wrote:
> > > > > > > > > ABataev wrote:
> > > > > > > > > > Why do we need all this stuff? I think the original code 
> > > > > > > > > > works good here, we just need to improve the message.
> > > > > > > > > > Why do we need all this stuff? I think the original code 
> > > > > > > > > > works good here, we just need to improve the message.
> > > > > > > > > 
> > > > > > > > > It seems we've been miscommunicating.  Let's review the 
> > > > > > > > > discussion so far, point by point, and I'll show you how I 
> > > > > > > > > arrived at this code.  I think the first major point is as 
> > > > > > > > > follows.
> > > > > > > > > 
> > > > > > > > > I asked:
> > > > > > > > > > Are you intentionally requiring support for __float128 when 
> > > > > > > > > > the source type is 128-bit long double? That seems to mean 
> > > > > > > > > > powerpc64le cannot offload to itself.
> > > > > > > > > 
> > > > > > > > > You replied:
> > > > > > > > > > No, if the host has 128 bit long double, the device must 
> > > > > > > > > > also have 128 bit long double. It has nothing to do with 
> > > > > > > > > > the float128 type itself.
> > > > > > > > > 
> > > > > > > > > I thought you were agreeing with my understanding.  That is, 
> > > > > > > > > the original code requires `__float128` support even when 
> > > > > > > > > 128-bit `long double` is in use.  That's why powerpc64le 
> > > > > > > > > cannot offload to itself.  How does the original code require 
> > > > > > > > > `__float128`?  It checks 
> > > > > > > > > `Context.getTargetInfo().hasFloat128Type()`.  As far as I can 
> > > > > > > > > tell, that checks for `__float128` support, and it does not 
> > > > > > > > > check for 128-bit `long double` support.  That's why 
> > > > > > > > > powerpc64le cannot offload to itself.
> > > > > > > > > 
> > > > > > > > > I'll review other points later so we can discuss them.  
> > > > > > > > > First, let's see if we can agree on this point.
> > > > > > > > > 
> > > > > > > > My point about ppc64le is:
> > > > > > > > If the host code uses long double 128 bit long, tbe device long 
> > > > > > > > double also must be 128 bit long. But if device does not 
> > > > > > > > support 128bit FP type naturally, user cannot do any operations 
> > > > > > > > with it except just load/stores.
> > > > > > > > Forget about float128 type, we talk about 128bit long double 
> > > > > > > > here.
> > > > > > > This patch adds new testing for powerpc64le.  Without the rest of 
> > > > > > > this patch (and without the `expected-error` change), should that 
> > > > > > > pass?  It does not for me.
> > > > > > Just like I said before, it means that your device config does not 
> > > > > > match the expected one. It means that either you specified 
> > > > > > incorrect triple, or the device is not configured properly.
> > > > > The triple is in the new test code.  Is it incorrect?
> > > > > 
> > > > > When you discussed device configuration earlier, you made a comment 
> > > > > about 64 bit FP, and I couldn't find evidence of that anywhere.
> > > > > 
> > > > > When I explore the device config as you suggested earlier, as far as 
> > > > > I can tell, the only issue here is that it claims powerpc64le does 
> > > > > not support `__float128`, but the original logic in 
> > > > > `Sema::checkOpenMPDeviceExpr` requires `__float128` by checking 
> > > > > `Context.getTargetInfo().hasFloat128Type()`.
> > > > Original ppc64le host reports that it supports 128bit float. The device 
> > > > reports that it does not support it. It just means that the device has 
> > > > different configuration than host. If yoh want the device to support 
> > > > 128 bit FP as the host, you need either to specify the device 
> > > > correctly, orfix the target device configuration in the 
> > > > lib/Basic/Target if it does not match the expected behavior. 
> > > > Check the host init for ppc64le. It sets hasFloat128 to true. For some 
> > > > reason, device target config does not set this flag and reports that it 
> > > > does not support 128bit FP. 
> > > > Original ppc64le host reports that it supports 128bit float.
> > > 
> > > > Check the host init for ppc64le. It sets hasFloat128 to true.
> > > 
> > > Doesn't appear to happen for me unless I add `-target-feature +float128` 
> > > as a `-cc1` option, as some existing powerpc64le clang tests do.
> > > 
> > > @MaskRay, will powerpc64le set `HasFloat128=true` after your changes?
> > > 
> > This is what I meant! We just missed some special target config options 

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596
 << Ty << E->getSourceRange();
+  if (Ty->isRealFloatingType()) {
+llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum(

ABataev wrote:
> jdenny wrote:
> > ABataev wrote:
> > > jdenny wrote:
> > > > ABataev wrote:
> > > > > jdenny wrote:
> > > > > > ABataev wrote:
> > > > > > > jdenny wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > jdenny wrote:
> > > > > > > > > > ABataev wrote:
> > > > > > > > > > > Why do we need all this stuff? I think the original code 
> > > > > > > > > > > works good here, we just need to improve the message.
> > > > > > > > > > > Why do we need all this stuff? I think the original code 
> > > > > > > > > > > works good here, we just need to improve the message.
> > > > > > > > > > 
> > > > > > > > > > It seems we've been miscommunicating.  Let's review the 
> > > > > > > > > > discussion so far, point by point, and I'll show you how I 
> > > > > > > > > > arrived at this code.  I think the first major point is as 
> > > > > > > > > > follows.
> > > > > > > > > > 
> > > > > > > > > > I asked:
> > > > > > > > > > > Are you intentionally requiring support for __float128 
> > > > > > > > > > > when the source type is 128-bit long double? That seems 
> > > > > > > > > > > to mean powerpc64le cannot offload to itself.
> > > > > > > > > > 
> > > > > > > > > > You replied:
> > > > > > > > > > > No, if the host has 128 bit long double, the device must 
> > > > > > > > > > > also have 128 bit long double. It has nothing to do with 
> > > > > > > > > > > the float128 type itself.
> > > > > > > > > > 
> > > > > > > > > > I thought you were agreeing with my understanding.  That 
> > > > > > > > > > is, the original code requires `__float128` support even 
> > > > > > > > > > when 128-bit `long double` is in use.  That's why 
> > > > > > > > > > powerpc64le cannot offload to itself.  How does the 
> > > > > > > > > > original code require `__float128`?  It checks 
> > > > > > > > > > `Context.getTargetInfo().hasFloat128Type()`.  As far as I 
> > > > > > > > > > can tell, that checks for `__float128` support, and it does 
> > > > > > > > > > not check for 128-bit `long double` support.  That's why 
> > > > > > > > > > powerpc64le cannot offload to itself.
> > > > > > > > > > 
> > > > > > > > > > I'll review other points later so we can discuss them.  
> > > > > > > > > > First, let's see if we can agree on this point.
> > > > > > > > > > 
> > > > > > > > > My point about ppc64le is:
> > > > > > > > > If the host code uses long double 128 bit long, tbe device 
> > > > > > > > > long double also must be 128 bit long. But if device does not 
> > > > > > > > > support 128bit FP type naturally, user cannot do any 
> > > > > > > > > operations with it except just load/stores.
> > > > > > > > > Forget about float128 type, we talk about 128bit long double 
> > > > > > > > > here.
> > > > > > > > This patch adds new testing for powerpc64le.  Without the rest 
> > > > > > > > of this patch (and without the `expected-error` change), should 
> > > > > > > > that pass?  It does not for me.
> > > > > > > Just like I said before, it means that your device config does 
> > > > > > > not match the expected one. It means that either you specified 
> > > > > > > incorrect triple, or the device is not configured properly.
> > > > > > The triple is in the new test code.  Is it incorrect?
> > > > > > 
> > > > > > When you discussed device configuration earlier, you made a comment 
> > > > > > about 64 bit FP, and I couldn't find evidence of that anywhere.
> > > > > > 
> > > > > > When I explore the device config as you suggested earlier, as far 
> > > > > > as I can tell, the only issue here is that it claims powerpc64le 
> > > > > > does not support `__float128`, but the original logic in 
> > > > > > `Sema::checkOpenMPDeviceExpr` requires `__float128` by checking 
> > > > > > `Context.getTargetInfo().hasFloat128Type()`.
> > > > > Original ppc64le host reports that it supports 128bit float. The 
> > > > > device reports that it does not support it. It just means that the 
> > > > > device has different configuration than host. If yoh want the device 
> > > > > to support 128 bit FP as the host, you need either to specify the 
> > > > > device correctly, orfix the target device configuration in the 
> > > > > lib/Basic/Target if it does not match the expected behavior. 
> > > > > Check the host init for ppc64le. It sets hasFloat128 to true. For 
> > > > > some reason, device target config does not set this flag and reports 
> > > > > that it does not support 128bit FP. 
> > > > > Original ppc64le host reports that it supports 128bit float.
> > > > 
> > > > > Check the host init for ppc64le. It sets hasFloat128 to true.
> > > > 
> > > > Doesn't appear to happen for me unless I add `-target-feature 
> > > > +float128` as a `-cc1` option, as some existing powerpc64le clang 

[PATCH] D63960: [C++20] Add consteval-specifique semantic

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

ping @rsmith


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63960/new/

https://reviews.llvm.org/D63960



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64078: [ASTImporter] Fix structural ineq of lambdas with different sloc

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,
it is a nice design question if source locations can participate in structural 
match or not. The comparison tells us that the same code written in different 
files is not structurally equivalent and I cannot agree with it. They can be 
not the same, but their structure is the same. The question is: why do we get 
to this comparison? Do we find a non-equivalent decl by lookup? I guess there 
can be another way to resolve this issue, and I'll be happy to help if you 
share what is the problem behind the patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64078/new/

https://reviews.llvm.org/D64078



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64075: [ASTImporter] Fix structural eq of lambdas

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,
The patch looks good, but it looks to me that it has a relation to 
https://reviews.llvm.org/D64078 that is kind of questionable to me. Let's delay 
landing this patch until the fix direction is clear.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64075/new/

https://reviews.llvm.org/D64075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62484: [ASTImporter] Added visibility context check for EnumDecl.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62484/new/

https://reviews.llvm.org/D62484



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60461: [ASTImporter] Import TemplateParameterLists in function templates.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Post-LGTM :)




Comment at: lib/AST/ASTImporter.cpp:2789
+  if (Num == 0)
+return Error::success();
+  SmallVector ToTPLists(Num);

Please add a newline after return.



Comment at: lib/AST/ASTImporter.cpp:2796
+else
+  return ToTPListOrErr.takeError();
+  ToD->setTemplateParameterListsInfo(Importer.ToContext, ToTPLists);

Let'a add a newline here too.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60461/new/

https://reviews.llvm.org/D60461



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63894: [CXX] Exercise all paths through these tests

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Ping. This is pretty straightforward, the only question is whether we want to 
preserve these older-dialect tests or rip them out.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63894/new/

https://reviews.llvm.org/D63894



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: rnk.

This is a better fix for the problem fixed in r334972.

Also remove the rm'ing of the symlink destination that was there to
clean up the bots -- it's over a year later, bots should be happy now.


https://reviews.llvm.org/D64301

Files:
  clang/test/Driver/no-canonical-prefixes.c


Index: clang/test/Driver/no-canonical-prefixes.c
===
--- clang/test/Driver/no-canonical-prefixes.c
+++ clang/test/Driver/no-canonical-prefixes.c
@@ -1,15 +1,13 @@
 // Due to ln -sf:
 // REQUIRES: shell
-// RUN: rm -rf %t.real
 // RUN: mkdir -p %t.real
 // RUN: cd %t.real
 // RUN: ln -sf %clang test-clang
 // RUN: cd ..
-// Important to remove %t.fake: If it already is a symlink to %t.real when
-// `ln -sf %t.real %t.fake` runs, then that would symlink %t.real to itself,
-// forming a cycle.
-// RUN: rm -rf %t.fake
-// RUN: ln -sf %t.real %t.fake
+// If %.fake already is a symlink to %t.real when `ln -sf %t.real %t.fake`
+// runs, then that would symlink %t.real to itself, forming a cycle.
+// The `-n` flag prevents this.
+// RUN: ln -sfn %t.real %t.fake
 // RUN: cd %t.fake
 // RUN: ./test-clang -v -S %s 2>&1 | FileCheck --check-prefix=CANONICAL %s
 // RUN: ./test-clang -v -S %s -no-canonical-prefixes 2>&1 | FileCheck 
--check-prefix=NON-CANONICAL %s


Index: clang/test/Driver/no-canonical-prefixes.c
===
--- clang/test/Driver/no-canonical-prefixes.c
+++ clang/test/Driver/no-canonical-prefixes.c
@@ -1,15 +1,13 @@
 // Due to ln -sf:
 // REQUIRES: shell
-// RUN: rm -rf %t.real
 // RUN: mkdir -p %t.real
 // RUN: cd %t.real
 // RUN: ln -sf %clang test-clang
 // RUN: cd ..
-// Important to remove %t.fake: If it already is a symlink to %t.real when
-// `ln -sf %t.real %t.fake` runs, then that would symlink %t.real to itself,
-// forming a cycle.
-// RUN: rm -rf %t.fake
-// RUN: ln -sf %t.real %t.fake
+// If %.fake already is a symlink to %t.real when `ln -sf %t.real %t.fake`
+// runs, then that would symlink %t.real to itself, forming a cycle.
+// The `-n` flag prevents this.
+// RUN: ln -sfn %t.real %t.fake
 // RUN: cd %t.fake
 // RUN: ./test-clang -v -S %s 2>&1 | FileCheck --check-prefix=CANONICAL %s
 // RUN: ./test-clang -v -S %s -no-canonical-prefixes 2>&1 | FileCheck --check-prefix=NON-CANONICAL %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r365288 - [clangd] Encapsulate fields in dex token. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Sun Jul  7 19:37:06 2019
New Revision: 365288

URL: http://llvm.org/viewvc/llvm-project?rev=365288&view=rev
Log:
[clangd] Encapsulate fields in dex token. NFC

Modified:
clang-tools-extra/trunk/clangd/index/dex/PostingList.h
clang-tools-extra/trunk/clangd/index/dex/Token.h

Modified: clang-tools-extra/trunk/clangd/index/dex/PostingList.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/PostingList.h?rev=365288&r1=365287&r2=365288&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/PostingList.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/PostingList.h Sun Jul  7 19:37:06 
2019
@@ -32,7 +32,7 @@
 namespace clang {
 namespace clangd {
 namespace dex {
-struct Token;
+class Token;
 
 /// NOTE: This is an implementation detail.
 ///

Modified: clang-tools-extra/trunk/clangd/index/dex/Token.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Token.h?rev=365288&r1=365287&r2=365288&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Token.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Token.h Sun Jul  7 19:37:06 2019
@@ -37,7 +37,8 @@ namespace dex {
 ///
 /// Tokens can be used to perform more sophisticated search queries by
 /// constructing complex iterator trees.
-struct Token {
+class Token {
+public:
   /// Kind specifies Token type which defines semantics for the internal
   /// representation. Each Kind has different representation stored in Data
   /// field.
@@ -76,10 +77,6 @@ struct Token {
 return TokenKind == Other.TokenKind && Data == Other.Data;
   }
 
-  /// Representation which is unique among Token with the same Kind.
-  std::string Data;
-  Kind TokenKind;
-
   friend llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Token &T) {
 switch (T.TokenKind) {
 case Kind::Trigram:
@@ -102,6 +99,10 @@ struct Token {
   }
 
 private:
+  /// Representation which is unique among Token with the same Kind.
+  std::string Data;
+  Kind TokenKind;
+
   friend llvm::hash_code hash_value(const Token &Token) {
 return llvm::hash_combine(static_cast(Token.TokenKind), Token.Data);
   }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r365289 - [clangd] Avoid slow ostreams in URI conversion.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Sun Jul  7 19:46:21 2019
New Revision: 365289

URL: http://llvm.org/viewvc/llvm-project?rev=365289&view=rev
Log:
[clangd] Avoid slow ostreams in URI conversion.

This speeds up some hot paths significantly (e.g.  dex::generateProximityURIs
by a third or so)

Modified:
clang-tools-extra/trunk/clangd/URI.cpp

Modified: clang-tools-extra/trunk/clangd/URI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/URI.cpp?rev=365289&r1=365288&r2=365289&view=diff
==
--- clang-tools-extra/trunk/clangd/URI.cpp (original)
+++ clang-tools-extra/trunk/clangd/URI.cpp Sun Jul  7 19:46:21 2019
@@ -14,8 +14,6 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include 
-#include 
-#include 
 
 LLVM_INSTANTIATE_REGISTRY(clang::clangd::URISchemeRegistry)
 
@@ -96,17 +94,16 @@ bool shouldEscape(unsigned char C) {
 /// - Unreserved characters are not escaped.
 /// - Reserved characters always escaped with exceptions like '/'.
 /// - All other characters are escaped.
-std::string percentEncode(llvm::StringRef Content) {
+void percentEncode(llvm::StringRef Content, std::string &Out) {
   std::string Result;
-  llvm::raw_string_ostream OS(Result);
   for (unsigned char C : Content)
 if (shouldEscape(C))
-  OS << '%' << llvm::format_hex_no_prefix(C, 2, /*Upper = */ true);
-else
-  OS << C;
-
-  OS.flush();
-  return Result;
+{
+  Out.push_back('%');
+  Out.push_back(llvm::hexdigit(C / 16));
+  Out.push_back(llvm::hexdigit(C % 16));
+} else
+{ Out.push_back(C); }
 }
 
 /// Decodes a string according to percent-encoding.
@@ -149,16 +146,18 @@ URI::URI(llvm::StringRef Scheme, llvm::S
 
 std::string URI::toString() const {
   std::string Result;
-  llvm::raw_string_ostream OS(Result);
-  OS << percentEncode(Scheme) << ":";
+  percentEncode(Scheme, Result);
+  Result.push_back(':');
   if (Authority.empty() && Body.empty())
-return OS.str();
+return Result;
   // If authority if empty, we only print body if it starts with "/"; 
otherwise,
   // the URI is invalid.
   if (!Authority.empty() || llvm::StringRef(Body).startswith("/"))
-OS << "//" << percentEncode(Authority);
-  OS << percentEncode(Body);
-  OS.flush();
+  {
+Result.append("//");
+percentEncode(Authority, Result);
+  }
+  percentEncode(Body, Result);
   return Result;
 }
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64305: [clangd] Add path mappings functionality

[William Wagner via Phabricator via cfe-commits]

wwagner19 created this revision.
wwagner19 added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
mgorny.
Herald added a project: clang.

Add path mappings to clangd which translate file URIs on inbound and outbound 
LSP messages. This mapping allows clangd to run in a remote environment (e.g. 
docker), where the source files and dependencies may be at different locations 
than the host. See 
http://lists.llvm.org/pipermail/clangd-dev/2019-January/000231.htm for more.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D64305

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/PathMapping.cpp
  clang-tools-extra/clangd/PathMapping.h
  clang-tools-extra/clangd/test/Inputs/path-mappings/compile_commands.json
  clang-tools-extra/clangd/test/Inputs/path-mappings/definition.jsonrpc
  clang-tools-extra/clangd/test/Inputs/path-mappings/remote/foo.cpp
  clang-tools-extra/clangd/test/Inputs/path-mappings/remote/foo.h
  clang-tools-extra/clangd/test/path-mappings.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/PathMappingTests.cpp

Index: clang-tools-extra/clangd/unittests/PathMappingTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/PathMappingTests.cpp
@@ -0,0 +1,132 @@
+//===-- PathMappingTests.cpp  *- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PathMapping.h"
+#include "llvm/Support/JSON.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using ::testing::ElementsAre;
+using ::testing::Pair;
+
+TEST(ParsePathMappingTests, ParseFailed) {
+  auto FailedParse = [](const std::vector &RawMappings) {
+auto Mappings = parsePathMappings(RawMappings);
+if (!Mappings) {
+  consumeError(Mappings.takeError());
+  return true;
+}
+return false;
+  };
+  // uneven mappings
+  EXPECT_TRUE(FailedParse({"/home/myuser1|"}));
+  // mappings need to be absolute
+  EXPECT_TRUE(FailedParse({"home/project|/workarea/project"}));
+  // improper delimiter
+  EXPECT_TRUE(FailedParse({"/home||/workarea"}));
+  // no delimiter
+  EXPECT_TRUE(FailedParse({"/home"}));
+}
+
+TEST(ParsePathMappingTests, AllowsWindowsAndUnixPaths) {
+  std::vector RawPathMappings = {
+  "/C:/home/project|/workarea/project",
+  "/home/project/.includes|/C:/opt/include"};
+  auto Parsed = parsePathMappings(RawPathMappings);
+  ASSERT_TRUE(bool(Parsed));
+  EXPECT_THAT(*Parsed,
+  ElementsAre(Pair("/C:/home/project", "/workarea/project"),
+  Pair("/home/project/.includes", "/C:/opt/include")));
+}
+
+TEST(ParsePathMappingTests, ParsesCorrectly) {
+  std::vector RawPathMappings = {
+  "/home/project|/workarea/project",
+  "/home/project/.includes|/opt/include"};
+  auto Parsed = parsePathMappings(RawPathMappings);
+  ASSERT_TRUE(bool(Parsed));
+  EXPECT_THAT(*Parsed,
+  ElementsAre(Pair("/home/project", "/workarea/project"),
+  Pair("/home/project/.includes", "/opt/include")));
+}
+
+TEST(DoPathMappingTests, PreservesOriginalParams) {
+  auto Params = llvm::json::parse(R"({
+"textDocument": {"uri": "file:///home/foo.cpp"},
+"position": {"line": 0, "character": 0}
+  })");
+  ASSERT_TRUE(bool(Params));
+  auto MappedParams =
+  doPathMapping(*Params, /*IsIncoming=*/true, /*Mappings=*/{});
+  EXPECT_EQ(MappedParams, *Params);
+}
+
+TEST(DoPathMappingTests, MapsUsingFirstMatch) {
+  auto Params = llvm::json::parse(R"({
+"textDocument": {"uri": "file:///home/project/foo.cpp"},
+"position": {"line": 0, "character": 0}
+})");
+  auto ExpectedParams = llvm::json::parse(R"({
+"textDocument": {"uri": "file:///workarea1/project/foo.cpp"},
+"position": {"line": 0, "character": 0}
+})");
+  ASSERT_TRUE(bool(Params) && bool(ExpectedParams));
+  PathMappings Mappings{{"/home", "/workarea1"}, {"/home", "/workarea2"}};
+  auto MappedParams = doPathMapping(*Params, /*IsIncoming=*/true, Mappings);
+  EXPECT_EQ(MappedParams, *ExpectedParams);
+}
+
+TEST(DoPathMappingTests, MapsOutgoing) {
+  auto Params = llvm::json::parse(R"({
+"result": "file:///opt/include/foo.h"
+})");
+  auto ExpectedParams = llvm::json::parse(R"({
+"result": "file:///home/project/.includes/foo.h"
+})");
+  ASSERT_TRUE(bool(Params) && bool(ExpectedParams));
+  PathMappings Mappings{{"/home/project/.includes", "/opt/include"}};
+  auto MappedParams = doPathMapping(*Params, /*IsIncoming=*/false, Ma

[PATCH] D64305: [clangd] Add path mappings functionality

[William Wagner via Phabricator via cfe-commits]

wwagner19 marked 2 inline comments as done.
wwagner19 added a comment.
Herald added a subscriber: ormris.

Hey,

This is my first proposed change to LLVM, so sorry if I messed anything up. The 
proposed changes here follow from discussion on clangd-dev (from janruary 
 and from 
june  ).
It seems like a rather large one, but fear not, most of the code is simply 
tests and wrapper code.

Happy to hear any feedback, thanks!




Comment at: clang-tools-extra/clangd/PathMapping.h:42
+/// untouched.
+llvm::json::Value doPathMapping(const llvm::json::Value &Params,
+bool IsIncoming, const PathMappings &Mappings);

Ideally this wouldn't be in the public interface, but I wanted to  unit test it 
and wasn't sure of a way to do that cleanly - other than putting it in the 
header.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:291
+   "opt/include"),
+llvm::cl::CommaSeparated);
 

Comma separated list here obviously limits the path mapping file paths, but 
there was precedent for this already (in `--QueryDriverGlobs`) and it seemed 
simplest. 

Also,a command-line argument felt the most straightforward, as I'm not aware of 
any clangd project settings file (please lmk if there is one :) ). Users can 
set up custom path mappings by using e.g. vscode workspace `settings.json`, 
coc.nvim `coc-settings.json`


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64305/new/

https://reviews.llvm.org/D64305



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63497: Add support for openSUSE RISC-V triple

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: lenary.

LGTM, thanks!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63497/new/

https://reviews.llvm.org/D63497



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64276: [ItaniumMangle] Refactor long double/__float128 mangling and fix the mangled code

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 208313.
MaskRay retitled this revision from "[ItaniumMangle] Replace 
useFloat128ManglingForLongDouble() with getManglingForLongDouble() and 
getManglingForFloat128()" to "[ItaniumMangle] Refactor long double/__float128 
mangling and fix the mangled code".
MaskRay edited the summary of this revision.
MaskRay added a comment.

Fix mangled code of __float128 on x86


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64276/new/

https://reviews.llvm.org/D64276

Files:
  include/clang/Basic/TargetInfo.h
  lib/AST/ItaniumMangle.cpp
  lib/Basic/Targets/PPC.h
  lib/Basic/Targets/SystemZ.h
  lib/Basic/Targets/X86.h
  test/CodeGenCXX/float128-declarations.cpp

Index: test/CodeGenCXX/float128-declarations.cpp
===
--- test/CodeGenCXX/float128-declarations.cpp
+++ test/CodeGenCXX/float128-declarations.cpp
@@ -84,15 +84,15 @@
 // CHECK-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global fp128 0xL40040800
 // CHECK-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x fp128]
 // CHECK-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x fp128] [fp128 0xL3FFF, fp128 0xL40008000, fp128 0xL4025176592E0]
-// CHECK-DAG: define internal fp128 @_ZN12_GLOBAL__N_16func1nERKU10__float128(fp128*
+// CHECK-DAG: define internal fp128 @_ZN12_GLOBAL__N_16func1nERKu9__ieee128(fp128*
 // CHECK-DAG: @f1f = global fp128 0xL
 // CHECK-DAG: @f2f = global fp128 0xL40040333
 // CHECK-DAG: @arr1f = global [10 x fp128]
 // CHECK-DAG: @arr2f = global [3 x fp128] [fp128 0xLBFFF, fp128 0xLC0008000, fp128 0xLC025176592E0]
-// CHECK-DAG: declare fp128 @_Z6func1fU10__float128(fp128)
-// CHECK-DAG: define linkonce_odr void @_ZN2C1C2EU10__float128(%class.C1* %this, fp128 %arg)
-// CHECK-DAG: define linkonce_odr fp128 @_ZN2C16func2cEU10__float128(fp128 %arg)
-// CHECK-DAG: define linkonce_odr fp128 @_Z6func1tIU10__float128ET_S0_(fp128 %arg)
+// CHECK-DAG: declare fp128 @_Z6func1fu9__ieee128(fp128)
+// CHECK-DAG: define linkonce_odr void @_ZN2C1C2Eu9__ieee128(%class.C1* %this, fp128 %arg)
+// CHECK-DAG: define linkonce_odr fp128 @_ZN2C16func2cEu9__ieee128(fp128 %arg)
+// CHECK-DAG: define linkonce_odr fp128 @_Z6func1tIu9__ieee128ET_S0_(fp128 %arg)
 // CHECK-DAG: @__const.main.s1 = private unnamed_addr constant %struct.S1 { fp128 0xL40060800 }
 // CHECK-DAG: store fp128 0xLF0AFD0EBFF292DCE42E0B38CDD83F26F, fp128* %f1l, align 16
 // CHECK-DAG: store fp128 0xL8000, fp128* %f2l, align 16
Index: lib/Basic/Targets/X86.h
===
--- lib/Basic/Targets/X86.h
+++ lib/Basic/Targets/X86.h
@@ -848,7 +848,7 @@
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
   }
 
-  bool useFloat128ManglingForLongDouble() const override { return true; }
+  const char *getMangledCodeOfLongDouble() const override { return "g"; }
 };
 } // namespace targets
 } // namespace clang
Index: lib/Basic/Targets/SystemZ.h
===
--- lib/Basic/Targets/SystemZ.h
+++ lib/Basic/Targets/SystemZ.h
@@ -141,7 +141,7 @@
 return "";
   }
 
-  bool useFloat128ManglingForLongDouble() const override { return true; }
+  const char *getMangledCodeOfLongDouble() const override { return "g"; }
 };
 } // namespace targets
 } // namespace clang
Index: lib/Basic/Targets/PPC.h
===
--- lib/Basic/Targets/PPC.h
+++ lib/Basic/Targets/PPC.h
@@ -314,10 +314,15 @@
 
   bool hasSjLjLowering() const override { return true; }
 
-  bool useFloat128ManglingForLongDouble() const override {
-return LongDoubleWidth == 128 &&
-   LongDoubleFormat == &llvm::APFloat::PPCDoubleDouble() &&
-   getTriple().isOSBinFormatELF();
+  const char *getMangledCodeOfLongDouble() const override {
+if (LongDoubleWidth == 64)
+  return "e";
+return LongDoubleFormat == &llvm::APFloat::PPCDoubleDouble()
+   ? "g"
+   : "u9__ieee128";
+  }
+  const char *getMangledCodeOfFloat128() const override {
+return "u9__ieee128";
   }
 };
 
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -2608,30 +2608,19 @@
 Out << 'd';
 break;
   case BuiltinType::LongDouble: {
-bool UseFloat128Mangling =
-getASTContext().getTargetInfo().useFloat128ManglingForLongDouble();
-if (getASTContext().getLangOpts().OpenMP &&
-getASTContext().getLangOpts().OpenMPIsDevice) {
-  UseFloat128Mangling = getASTContext()
-.getAuxTargetInfo()
-  

[PATCH] D64277: [X86][PowerPC] Support -mlong-double-128

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 208314.
MaskRay added a comment.

Rebase


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64277/new/

https://reviews.llvm.org/D64277

Files:
  include/clang/Driver/Options.td
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/PPC.cpp
  lib/Basic/Targets/X86.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/ppc64-long-double.cpp
  test/CodeGen/x86-long-double.cpp
  test/Driver/mlong-double-128.c
  test/OpenMP/nvptx_unsupported_type_codegen.cpp

Index: test/OpenMP/nvptx_unsupported_type_codegen.cpp
===
--- test/OpenMP/nvptx_unsupported_type_codegen.cpp
+++ test/OpenMP/nvptx_unsupported_type_codegen.cpp
@@ -76,6 +76,3 @@
   f = 1;
   return f;
 }
-
-// CHECK: define weak void @__omp_offloading_{{.+}}foo{{.+}}_l75([[BIGTYPE:.+]]*
-// CHECK: store [[BIGTYPE]] 0xL3FFF, [[BIGTYPE]]* %
Index: test/Driver/mlong-double-128.c
===
--- /dev/null
+++ test/Driver/mlong-double-128.c
@@ -0,0 +1,11 @@
+// RUN: %clang -target powerpc-linux-musl -c -### %s -mlong-double-128 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64-pc-freebsd12 -c -### %s -mlong-double-128 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64le-linux-musl -c -### %s -mlong-double-128 2>&1 | FileCheck %s
+// RUN: %clang -target i686-linux-gnu -c -### %s -mlong-double-128 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-musl -c -### %s -mlong-double-128 2>&1 | FileCheck %s
+
+// CHECK: "-mlong-double-128"
+
+// RUN: %clang -target aarch64 -c -### %s -mlong-double-128 2>&1 | FileCheck --check-prefix=ERR %s
+
+// ERR: error: unsupported option '-mlong-double-128' for target 'aarch64'
Index: test/CodeGen/x86-long-double.cpp
===
--- test/CodeGen/x86-long-double.cpp
+++ test/CodeGen/x86-long-double.cpp
@@ -16,6 +16,15 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin -mlong-double-64 | \
 // RUN:   FileCheck --check-prefixes=FP64,FP64-X64 %s
 
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686 -mlong-double-128 | \
+// RUN:   FileCheck --check-prefix=FP128 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin -mlong-double-128 | \
+// RUN:   FileCheck --check-prefix=FP128 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64 -mlong-double-128 | \
+// RUN:   FileCheck --check-prefix=FP128 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-apple-darwin -mlong-double-128 | \
+// RUN:   FileCheck --check-prefix=FP128 %s
+
 // Check -malign-double increases the alignment from 4 to 8 on x86-32.
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686 -mlong-double-64 \
 // RUN:   -malign-double | FileCheck --check-prefixes=FP64,FP64-X64 %s
@@ -37,7 +46,11 @@
 // FP64-X64: @x = global double {{.*}}, align 8
 // FP64-X64: @size = global i32 8
 
+// FP128: @x = global fp128 {{.*}}, align 16
+// FP128: @size = global i32 16
+
 long double foo(long double d) { return d; }
 
 // FP64: double @_Z3fooe(double %d)
 // FP80: x86_fp80 @_Z3fooe(x86_fp80 %d)
+// FP128: fp128 @_Z3foog(fp128 %d)
Index: test/CodeGen/ppc64-long-double.cpp
===
--- test/CodeGen/ppc64-long-double.cpp
+++ test/CodeGen/ppc64-long-double.cpp
@@ -2,8 +2,11 @@
 // RUN:   FileCheck --check-prefix=FP64 %s
 // RUN: %clang_cc1 -triple powerpc64-linux-gnu -emit-llvm -o - %s -mlong-double-64 | \
 // RUN:   FileCheck --check-prefix=FP64 %s
+
 // RUN: %clang_cc1 -triple powerpc64-linux-gnu -emit-llvm -o - %s | \
 // RUN:   FileCheck --check-prefix=IBM128 %s
+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - -mlong-double-128 %s | \
+// RUN:   FileCheck --check-prefix=IBM128 %s
 
 long double x = 0;
 int size = sizeof(x);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2742,7 +2742,9 @@
   Opts.PackStruct = getLastArgIntValue(Args, OPT_fpack_struct_EQ, 0, Diags);
   Opts.MaxTypeAlign = getLastArgIntValue(Args, OPT_fmax_type_align_EQ, 0, Diags);
   Opts.AlignDouble = Args.hasArg(OPT_malign_double);
-  Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
+  Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_128)
+? 128
+: Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
   Opts.PICLevel = getLastArgIntValue(Args, OPT_pic_level, 0, Diags);
   Opts.ROPI = Args.hasArg(OPT_fropi);
   Opts.RWPI = Args.hasArg(OPT_frwpi);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4001,11 +4001,14 @@
 
   RenderFloatingPointOptions(TC, D, OFastEnabled,

[PATCH] D64305: [clangd] Add path mappings functionality

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for putting this together. Overall it looks pretty good!

Main issues to address are:

- file path handling should manage windows paths and have tests for this
  - the lit test can be simplified quite a lot I think




Comment at: clang-tools-extra/clangd/PathMapping.cpp:29
+  using Kind = llvm::json::Value::Kind;
+  const auto &K = V.kind();
+  if (K == Kind::Object) {

V.kind() is an enum, using `const auto&` here is confusing. It's just `Kind`?



Comment at: clang-tools-extra/clangd/PathMapping.cpp:30
+  const auto &K = V.kind();
+  if (K == Kind::Object) {
+for (auto &KV : *V.getAsObject()) {

object keys may be file uris too. (see `WorkspaceEdit.changes`)

This case is going to be a bit annoying to handle, I think :-(



Comment at: clang-tools-extra/clangd/PathMapping.cpp:51
+llvm::json::Value MappedParams =
+doPathMapping(Params, /*IsIncoming=*/true, Mappings);
+return WrappedHandler.onNotify(Method, std::move(MappedParams));

not a really big deal, but we're forcing a copy here - should doPathMapping 
mutate its argument and return void instead?



Comment at: clang-tools-extra/clangd/PathMapping.cpp:123
+parsePathMappings(const std::vector &RawPathMappings) {
+  if (!RawPathMappings.size()) {
+return make_string_error("Must provide at least one path mapping");

I'm not sure why this needs to be an error



Comment at: clang-tools-extra/clangd/PathMapping.cpp:135
+  return make_string_error("Path mapping not absolute: " + HostPath);
+} else if (!llvm::sys::path::is_absolute(RemotePath)) {
+  return make_string_error("Path mapping not absolute: " + RemotePath);

will an absolute windows path `C:\foo` be treated as absolute on a unix machine?
(I know the reverse is true)



Comment at: clang-tools-extra/clangd/PathMapping.cpp:142
+  llvm::raw_string_ostream OS(S);
+  OS << "Parsed path mappings: ";
+  for (const auto &P : ParsedMappings)

Please leave this to the caller to log, if necessary.

(I think we're likely to start logging argv/config on startup, so in general 
having every flag logged separately probably isn't desirable)



Comment at: clang-tools-extra/clangd/PathMapping.cpp:154
+  recursivelyMap(
+  MappedParams, [&Mappings, IsIncoming](llvm::StringRef S) -> std::string {
+if (!S.startswith("file://"))

again, copying every string in the json seems excessive. can we return 
optional and only populate it when we match?



Comment at: clang-tools-extra/clangd/PathMapping.cpp:163
+for (const auto &Mapping : Mappings) {
+  const auto &From = IsIncoming ? Mapping.first : Mapping.second;
+  const auto &To = IsIncoming ? Mapping.second : Mapping.first;

auto here is just string, I think?



Comment at: clang-tools-extra/clangd/PathMapping.cpp:165
+  const auto &To = IsIncoming ? Mapping.second : Mapping.first;
+  if (Uri->body().startswith(From)) {
+std::string MappedBody = Uri->body();

This is simplistic I'm afraid: it's not going to work at all on windows (where 
paths have `\` but URIs have `/`), and it's going to falsely match 
"/foo-bar/baz" against a mapping for "/foo".

`llvm::sys::fs::replace_path_prefix` is probably the beginning of a solution. 
If I'm reading correctly the first two args need to have the same slash style 
and OldPrefix should have its trailing slash.

I'd actually suggest pulling out the function to map one string, and 
unit-testing that, to catch all the filename cases.

Then the json::Value traversal tests should be focused on testing the places 
where a string can appear in JSON, not all the different contents of the string.



Comment at: clang-tools-extra/clangd/PathMapping.cpp:169
+auto MappedUri = URI(Uri->scheme(), Uri->authority(), MappedBody);
+vlog("Mapped {0} file path from {1} to {2}",
+ IsIncoming ? "incoming" : "outgoing", Uri->toString(),

This is too verbose for vlog. If you think it's important to keep, it should be 
dlog.



Comment at: clang-tools-extra/clangd/PathMapping.h:21
+
+/// PathMappings are a collection of paired host and remote paths.
+/// These pairs are used to alter file:// URIs appearing in inbound and 
outbound

hmm, the host/remote terminology is a bit confusing to me.
I guess the host is the machine where clangd is running, and remote is the 
other end, but  from the user's point of view this is a configuration where the 
clangd is remote.

What about calling these client/server paths? The language client/server roles 
are defined in the spec and these terms are likely to make some sense to users.


==

[PATCH] D64283: [PowerPC] Support -mabi=ieeelongdouble and -mabi=ibmlongdouble

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 208315.
MaskRay edited the summary of this revision.
MaskRay added a comment.
Herald added subscribers: krytarowski, emaste.

Update description.

Fix the mangled code: U10__float128 -> u9__ieee128


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64283/new/

https://reviews.llvm.org/D64283

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/PPC.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/ppc64-long-double.cpp
  test/Driver/ppc-abi.c

Index: test/Driver/ppc-abi.c
===
--- test/Driver/ppc-abi.c
+++ test/Driver/ppc-abi.c
@@ -66,4 +66,20 @@
 // CHECK-ELFv2-PIC: "-mrelocation-model" "pic" "-pic-level" "2"
 // CHECK-ELFv2-PIC: "-target-abi" "elfv2"
 
+// Check -mabi=ieeelongdouble is passed through but it does not change -target-abi.
+// RUN: %clang -target powerpc64le-linux-gnu %s -mabi=ieeelongdouble -mabi=elfv1 -### 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ELFv1-IEEE %s
+// RUN: %clang -target powerpc64le-linux-gnu %s -mabi=elfv1 -mabi=ieeelongdouble -### 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ELFv1-IEEE %s
 
+// CHECK-ELFv1-IEEE: "-mabi=ieeelongdouble"
+// CHECK-ELFv1-IEEE: "-target-abi" "elfv1"
+
+// Check -mabi=ibmlongdouble is the default.
+// RUN: %clang -target powerpc64le-linux-gnu %s -### 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ELFv2-IBM128 %s
+// RUN: %clang -target powerpc64le-linux-gnu %s -mabi=ibmlongdouble -### 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ELFv2-IBM128 %s
+
+// CHECK-ELFv2-IBM128-NOT: "-mabi=ieeelongdouble"
+// CHECK-ELFv2-IBM128: "-target-abi" "elfv2"
Index: test/CodeGen/ppc64-long-double.cpp
===
--- test/CodeGen/ppc64-long-double.cpp
+++ test/CodeGen/ppc64-long-double.cpp
@@ -3,6 +3,14 @@
 // RUN: %clang_cc1 -triple powerpc64-linux-gnu -emit-llvm -o - %s -mlong-double-64 | \
 // RUN:   FileCheck --check-prefix=FP64 %s
 
+// musl defaults to -mlong-double-64, so -mlong-double-128 is needed to make
+// -mabi=ieeelongdouble effective.
+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - %s -mlong-double-128 \
+// RUN:   -mabi=ieeelongdouble | FileCheck --check-prefix=FP128 %s
+// RUN: %clang_cc1 -triple powerpc64-linux-gnu -emit-llvm -o - %s \
+// RUN:   -mabi=ieeelongdouble | FileCheck --check-prefix=FP128 %s
+
+// IBM extended double is the default.
 // RUN: %clang_cc1 -triple powerpc64-linux-gnu -emit-llvm -o - %s | \
 // RUN:   FileCheck --check-prefix=IBM128 %s
 // RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - -mlong-double-128 %s | \
@@ -13,10 +21,13 @@
 
 // FP64: @x = global double {{.*}}, align 8
 // FP64: @size = global i32 8
+// FP128: @x = global fp128 {{.*}}, align 16
+// FP128: @size = global i32 16
 // IBM128: @x = global ppc_fp128 {{.*}}, align 16
 // IBM128: @size = global i32 16
 
 long double foo(long double d) { return d; }
 
 // FP64: double @_Z3fooe(double %d)
+// FP128: fp128 @_Z3foou9__ieee128(fp128 %d)
 // IBM128: ppc_fp128 @_Z3foog(ppc_fp128 %d)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2745,6 +2745,7 @@
   Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_128)
 ? 128
 : Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
+  Opts.PPCIEEELongDouble = Args.hasArg(OPT_mabi_EQ_ieeelongdouble);
   Opts.PICLevel = getLastArgIntValue(Args, OPT_pic_level, 0, Diags);
   Opts.ROPI = Args.hasArg(OPT_fropi);
   Opts.RWPI = Args.hasArg(OPT_frwpi);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1808,12 +1808,27 @@
   break;
 }
 
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
-// The ppc64 linux abis are all "altivec" abis by default. Accept and ignore
-// the option if given as we don't have backend support for any targets
-// that don't use the altivec abi.
-if (StringRef(A->getValue()) != "altivec")
-  ABIName = A->getValue();
+  bool IEEELongDouble = false, SeenLongDouble = false, SeenOther = false;
+  for (Arg *A : Args.filtered_reverse(options::OPT_mabi_EQ,
+  options::OPT_mabi_EQ_ieeelongdouble)) {
+if (A->getOption().matches(options::OPT_mabi_EQ_ieeelongdouble)) {
+  A->claim();
+  if (!SeenLongDouble)
+IEEELongDouble = true;
+  SeenLongDouble = true;
+} else if (StringRef(A->getValue()) == "ibmlongdouble") {
+  SeenLongDouble = true;
+} else if (StringRef(A->getValue()) != "altivec") {
+  // The ppc64 linux abis are all "altivec" abis by default. Accept and ignore
+  // t

[PATCH] D64307: [clangd] Add a missing early return in getTypeHierarchy()

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64307

Files:
  clang-tools-extra/clangd/XRefs.cpp


Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1191,6 +1191,8 @@
   RecursionProtectionSet RPSet;
   Optional Result =
   getTypeAncestors(*CXXRD, AST.getASTContext(), RPSet);
+  if (!Result)
+return Result;
 
   if ((Direction == TypeHierarchyDirection::Children ||
Direction == TypeHierarchyDirection::Both) &&


Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1191,6 +1191,8 @@
   RecursionProtectionSet RPSet;
   Optional Result =
   getTypeAncestors(*CXXRD, AST.getASTContext(), RPSet);
+  if (!Result)
+return Result;
 
   if ((Direction == TypeHierarchyDirection::Children ||
Direction == TypeHierarchyDirection::Both) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64308: [clangd] Implement typeHierarchy/resolve for subtypes

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

This allows the client to resolve subtypes one level at a time.

For supertypes, this is not necessary, because we eagerly compute
supertypes and return all levels.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64308

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/type-hierarchy.test

Index: clang-tools-extra/clangd/test/type-hierarchy.test
===
--- clang-tools-extra/clangd/test/type-hierarchy.test
+++ clang-tools-extra/clangd/test/type-hierarchy.test
@@ -1,7 +1,7 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent {};\nstruct Child1 : Parent {};\nstruct Child2 : Child1 {};\nstruct Child3 : Child2 {};"}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"struct Parent {};\nstruct Child1 : Parent {};\nstruct Child2 : Child1 {};\nstruct Child3 : Child2 {};\nstruct Child4 : Child3 {};"}}}
 ---
 {"jsonrpc":"2.0","id":1,"method":"textDocument/typeHierarchy","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":11},"direction":2,"resolve":1}}
 #  CHECK:  "id": 1
@@ -9,6 +9,7 @@
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:"children": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"data": "A6576FE083F2949A",
 # CHECK-NEXT:"kind": 23,
 # CHECK-NEXT:"name": "Child3",
 # CHECK-NEXT:"range": {
@@ -114,6 +115,64 @@
 # CHECK-NEXT:"uri": "file:///clangd-test/main.cpp"
 # CHECK-NEXT:  }
 ---
-{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+{"jsonrpc":"2.0","id":2,"method":"typeHierarchy/resolve","params":{"item":{"uri":"test:///main.cpp","data":"A6576FE083F2949A","name":"Child3","kind":23,"range":{"end":{"character":13,"line":3},"start":{"character":7,"line":3}},"selectionRange":{"end":{"character":13,"line":3},"start":{"character":7,"line":3}}},"direction":0,"resolve":1}}
+#  CHECK:  "id": 2
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"children": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"data": "5705B382DFC77CBC",
+# CHECK-NEXT:"kind": 23,
+# CHECK-NEXT:"name": "Child4",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 13,
+# CHECK-NEXT:"line": 4
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 4
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"selectionRange": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 13,
+# CHECK-NEXT:"line": 4
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 4
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"uri": "file:///clangd-test/main.cpp"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"data": "A6576FE083F2949A",
+# CHECK-NEXT:"kind": 23,
+# CHECK-NEXT:"name": "Child3",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 13,
+# CHECK-NEXT:"line": 3
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 3
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"selectionRange": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 13,
+# CHECK-NEXT:"line": 3
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 3
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"uri": "file:///clangd-test/main.cpp"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/XRefs.h
===
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -140,6 +140,11 @@
 ParsedAST &AST, Position Pos, int Resolve, TypeHierarchyDirection Direction,
 const SymbolIndex *Index = nullptr, PathRef TUPat

[PATCH] D64308: [clangd] Implement typeHierarchy/resolve for subtypes

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

This was an oversight on my part: I forgot that, when we didn't implement 
typeHierarchy/resolve, it was only supertypes that didn't need it; subtypes do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64308/new/

https://reviews.llvm.org/D64308



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits