nemanjai closed this revision.
nemanjai added a comment.
Committed revision 266186.
Repository:
rL LLVM
http://reviews.llvm.org/D15120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: lib/Sema/SemaOverload.cpp:1966-1967
@@ -1955,4 +1965,4 @@
// C99 6.3.1.5p1:
// When a float is promoted to double or long double, or a
hubert.reinterpretcast added inline comments.
Comment at: lib/Sema/SemaOverload.cpp:1658
@@ -1657,1 +1657,3 @@
} else if (FromType->isRealFloatingType() && ToType->isRealFloatingType()) {
+// FIXME: disable conversions between long double and __float128 if
+// their rep
nemanjai added inline comments.
Comment at: lib/Sema/SemaOverload.cpp:1658
@@ -1657,1 +1657,3 @@
} else if (FromType->isRealFloatingType() && ToType->isRealFloatingType()) {
+// FIXME: disable conversions between long double and __float128 if
+// their representation is
hubert.reinterpretcast added inline comments.
Comment at: lib/Sema/SemaOverload.cpp:1658
@@ -1657,1 +1657,3 @@
} else if (FromType->isRealFloatingType() && ToType->isRealFloatingType()) {
+// FIXME: disable conversions between long double and __float128 if
+// their rep
nemanjai updated this revision to Diff 49508.
nemanjai added a comment.
Removed questionable macro definitions.
Renamed the test function for invalid conversions and changed the semantics so
that it allows __float128 <-> long double conversions only if the two types
have the same representation
hubert.reinterpretcast added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:718
@@ -717,1 +717,3 @@
+ if (TI.hasFloat128Type())
+DefineFloatMacros(Builder, "FLT128", &TI.getFloat128Format(), "Q");
I am concerned that setting these macros are
hubert.reinterpretcast added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:1169
@@ +1168,3 @@
+ if (LHSElemType == S.Context.Float128Ty &&
+ RHSElemType == S.Context.LongDoubleTy)
+return true;
nemanjai wrote:
> hubert.reinterpretcast wrote:
> > I d
nemanjai added inline comments.
Comment at: lib/Basic/TargetInfo.cpp:231
@@ +230,3 @@
+if (hasFloat128Type() &&
+&getFloat128Format() == &llvm::APFloat::IEEEquad)
+ return Float128;
hubert.reinterpretcast wrote:
> Is it necessary to check that `__
hubert.reinterpretcast added inline comments.
Comment at: lib/Basic/TargetInfo.cpp:231
@@ +230,3 @@
+if (hasFloat128Type() &&
+&getFloat128Format() == &llvm::APFloat::IEEEquad)
+ return Float128;
Is it necessary to check that `__float128` is IEEE
nemanjai updated this revision to Diff 47223.
nemanjai added a comment.
Fixed the naming, indentation and removed the use of dyn_cast for types.
Repository:
rL LLVM
http://reviews.llvm.org/D15120
Files:
bindings/python/clang/cindex.py
include/clang-c/Index.h
include/clang/AST/ASTContex
On Wed, Feb 3, 2016 at 3:52 PM, Hubert Tong <
hubert.reinterpretc...@gmail.com> wrote:
> hubert.reinterpretcast added inline comments.
>
>
> Comment at: lib/Sema/SemaExpr.cpp:1156-1159
> @@ +1155,6 @@
> +
> + QualType LHSElemType = dyn_cast(LHSType) ?
> +cast(LHSType)->getEle
hubert.reinterpretcast added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:1156-1159
@@ +1155,6 @@
+
+ QualType LHSElemType = dyn_cast(LHSType) ?
+cast(LHSType)->getElementType() : LHSType;
+ QualType RHSElemType = dyn_cast(RHSType) ?
+cast(RHSType)->getElementType(
rsmith added inline comments.
Comment at: include/clang/Driver/Options.td:1463
@@ +1462,3 @@
+Group;
+def mnofloat128 : Flag<["-"], "mno-float128">,
+Group;
Per the mangling rules at the top of this file, this should be named
`mno_float128`.
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#340515, @nemanjai wrote:
> If the reviewers don't mind, I would like to keep this patch with diagnostics
> for interoperability between the two types for now. This is simply because
> enabling such interoperability requir
nemanjai added a comment.
If the reviewers don't mind, I would like to keep this patch with diagnostics
for interoperability between the two types for now. This is simply because
enabling such interoperability requires changes to some of the conversion
infrastructure (i.e. allowing FPTrunc/FPEx
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#337144, @nemanjai wrote:
> Do you suggest that we need to allow operations (or at least assignments)
> between the two types and take away the diagnostics that are part of this
> patch?
It appears that the conclusion is
rjmccall added a comment.
In http://reviews.llvm.org/D15120#337975, @hubert.reinterpretcast wrote:
> In http://reviews.llvm.org/D15120#337654, @rjmccall wrote:
>
> > In http://reviews.llvm.org/D15120#337552, @hubert.reinterpretcast wrote:
> >
> > > It remains that the present standardization effo
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#337654, @rjmccall wrote:
> In http://reviews.llvm.org/D15120#337552, @hubert.reinterpretcast wrote:
>
> > It remains that the present standardization effort (as `_Float128`) does
> > not imbue the "interchange" type with i
rjmccall added a comment.
In http://reviews.llvm.org/D15120#337552, @hubert.reinterpretcast wrote:
> In http://reviews.llvm.org/D15120#337384, @rjmccall wrote:
>
> > I think it's not unlikely that float128_t will see future standardization
> > (as an optional extension, of course), and it would
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#337384, @rjmccall wrote:
> I think it's not unlikely that float128_t will see future standardization (as
> an optional extension, of course), and it would be strange for an operation
> between two types to not consistentl
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#337384, @rjmccall wrote:
> I think it's not unlikely that float128_t will see future standardization (as
> an optional extension, of course), and it would be strange for an operation
> between two types to not consistentl
rjmccall added a comment.
I think it's not unlikely that float128_t will see future standardization (as
an optional extension, of course), and it would be strange for an operation
between two types to not consistently yield the type of higher rank.
I could see an argument that we should not tre
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#337144, @nemanjai wrote:
> Do you suggest that we need to allow operations (or at least assignments)
> between the two types and take away the diagnostics that are part of this
> patch?
My overriding concern at this tim
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#337144, @nemanjai wrote:
> Do you suggest that we need to allow operations (or at least assignments)
> between the two types and take away the diagnostics that are part of this
> patch?
My overriding concern at this tim
nemanjai added a comment.
Thank you for the discussion on rank. I'm glad we have an agreement on the rank
of the two types.
However, considering __float128 already has a higher rank in this patch, is
there anything else that you would like me to change here before the patch is
approved?
Do you
rjmccall added a comment.
In http://reviews.llvm.org/D15120#336996, @hubert.reinterpretcast wrote:
> In http://reviews.llvm.org/D15120#336891, @rjmccall wrote:
>
> > > The C committee decided that "undefined behavior" was what they could
> > > agree on for this sort of case.
> >
> >
> > That's o
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#336891, @rjmccall wrote:
> > The C committee decided that "undefined behavior" was what they could agree
> > on for this sort of case.
>
>
> That's only when the operand value is actually outside of the range of the
> typ
rjmccall added a comment.
In http://reviews.llvm.org/D15120#336855, @hubert.reinterpretcast wrote:
> In http://reviews.llvm.org/D15120#336827, @rjmccall wrote:
>
> > Here's the thing, though: I don't think there's a reasonable language
> > solution here besides saying that float128_t has higher
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#336827, @rjmccall wrote:
> Here's the thing, though: I don't think there's a reasonable language
> solution here besides saying that float128_t has higher rank. You can't make
> the types incompatible, because it's clear
rjmccall added a comment.
In http://reviews.llvm.org/D15120#336776, @hubert.reinterpretcast wrote:
> In http://reviews.llvm.org/D15120#336430, @rjmccall wrote:
>
> > As I understand it, PPC's long-double (~103 bits of precision) is still
> > strictly less precise than float128_t (113 bits of pre
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#336430, @rjmccall wrote:
> As I understand it, PPC's long-double (~103 bits of precision) is still
> strictly less precise than float128_t (113 bits of precision), so it ought to
> be have lower rank. Is there actually a
rjmccall added a comment.
In http://reviews.llvm.org/D15120#336282, @nemanjai wrote:
> Addressed review comments.
> The key differences are:
>
> - No assignments or operations between entities of long double and __float128
> allowed if the two types have a different representation
> - Each type
nemanjai updated this revision to Diff 45998.
nemanjai added a comment.
Addressed review comments.
The key differences are:
- No assignments or operations between entities of long double and __float128
allowed if the two types have a different representation
- Each type has a distinct rank
This
hubert.reinterpretcast added inline comments.
Comment at: lib/AST/ASTContext.cpp:4635
@@ +4634,3 @@
+ // If the two sides have Float128Rank and LongDoubleRank and the two types
+ // have the same representation on this platform, they have equal rank.
+ if ((LHSR == LongDoubleRa
rjmccall added inline comments.
Comment at: include/clang/Basic/TargetInfo.h:385
@@ +384,3 @@
+ unsigned getFloat128Width() const { return Float128Width; }
+ unsigned getFloat128Align() const { return Float128Align; }
+ const llvm::fltSemantics &getFloat128Format() const {
hubert.reinterpretcast added inline comments.
Comment at: lib/CodeGen/CGDebugInfo.cpp:550
@@ -548,1 +549,3 @@
case BuiltinType::Double:
+// FIXME: For targets where lond double and __float128 have the same size,
+// they are currently indistinguishable in the debugger w
nemanjai updated the summary for this revision.
nemanjai updated this revision to Diff 44459.
nemanjai added a comment.
Addressed the review comments:
- Added the requested test cases
- Disabled promotion of long double to __float128
- Disabled operations between long double and __float128 if the
nemanjai added a comment.
I think the correct course of action would be to allow/disallow promotion based
on a condition that the two types are the same/different (respectively). I
think a comparison of the float semantics is a valid way to check this. Also,
should operations between long doubl
hubert.reinterpretcast added a comment.
In http://reviews.llvm.org/D15120#310992, @nemanjai wrote:
> I think the correct course of action would be to allow/disallow promotion
> based on a condition that the two types are the same/different
> (respectively). I think a comparison of the float sem
hubert.reinterpretcast added a comment.
I would also like to see a test case based on the following from ISO/IEC TS
18661-3:2015:
> If both operands have floating types and neither of the sets of values of
> their corresponding real types is a subset of (or equivalent to) the other,
> the beha
nemanjai added inline comments.
Comment at: include/clang/Basic/TargetInfo.h:384
@@ +383,3 @@
+ unsigned getFloat128Width() const { return 128; }
+ unsigned getFloat128Align() const { return 128; }
+ const llvm::fltSemantics &getFloat128Format() const {
hubert.
hubert.reinterpretcast added a comment.
I would like to see a test:
__float128 qf();
long double ldf();
long double ld{qf()}; // error: narrowing
__float128 q{ldf()}; // passes
Comment at: include/clang/Basic/TargetInfo.h:384
@@ +383,3 @@
+ unsigned getFloat128Width
nemanjai added inline comments.
Comment at: lib/AST/ItaniumMangle.cpp:2064
@@ +2063,3 @@
+if (getASTContext().getTargetInfo().useFloat128ManglingForLongDouble())
+ Out << "U10__float128"; // Match the GCC mangling
+else
Please refer to https://gcc.gnu
nemanjai updated this revision to Diff 42414.
nemanjai added a comment.
Updated to address review comments. Rather than listing responses and
justification for some of the changes, I'll provide comments inline for better
readability.
Repository:
rL LLVM
http://reviews.llvm.org/D15120
Files
> On Dec 7, 2015, at 12:16 AM, John McCall wrote:
>
> rjmccall added a reviewer: akyrtzi.
>
>
> Comment at: lib/AST/ASTContext.cpp:5424
> @@ -5410,1 +5423,3 @@
> +// FIXME: need to figure out what this is for __float128
> +case BuiltinType::Float128: return 'K';
>
rjmccall added a reviewer: akyrtzi.
Comment at: lib/AST/ASTContext.cpp:5424
@@ -5410,1 +5423,3 @@
+// FIXME: need to figure out what this is for __float128
+case BuiltinType::Float128: return 'K';
case BuiltinType::NullPtr:return '*'; // like char*
-
rsmith added a comment.
+rjmccall for `@encode` and USR mangling.
Comment at: include/clang-c/Index.h:2879-2885
@@ -2878,8 +2878,9 @@
CXType_LongDouble = 23,
- CXType_NullPtr = 24,
- CXType_Overload = 25,
- CXType_Dependent = 26,
- CXType_ObjCId = 27,
- CXType_ObjCClass
nemanjai added a comment.
Just a friendly reminder to the reviewers that this patch has been up for
almost a week with no review comments. Please take some time to review.
Repository:
rL LLVM
http://reviews.llvm.org/D15120
___
cfe-commits mailin
nemanjai created this revision.
nemanjai added reviewers: hfinkel, wschmidt, kbarton, rsmith.
nemanjai added a subscriber: cfe-commits.
nemanjai set the repository for this revision to rL LLVM.
Herald added a subscriber: klimek.
This patch adds support for the __float128 keyword and literals for I
50 matches
Mail list logo
