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

Reply via email to