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
