[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
efriedma added a comment. There's nothing wrong with this change, as far as I can tell. That said, if you're planning to use getRealTypeByWidth anywhere other than AddModeAttr, it's probably a bug; the behavior of getRealTypeByWidth isn't useful for any other purpose given that clang supports multiple 128-bit floating-point types. https://reviews.llvm.org/D26955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
ddcc updated this revision to Diff 79996.
ddcc added a comment.
Change definition
https://reviews.llvm.org/D26955
Files:
lib/Basic/TargetInfo.cpp
lib/Sema/SemaDeclAttr.cpp
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -3388,7 +3388,7 @@
DestWidth = 64;
break;
case 'X':
- DestWidth = 96;
+ DestWidth = 80;
break;
case 'T':
DestWidth = 128;
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -226,7 +226,7 @@
return Double;
switch (BitWidth) {
- case 96:
+ case 80:
if (() == ::APFloat::x87DoubleExtended)
return LongDouble;
break;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -3388,7 +3388,7 @@
DestWidth = 64;
break;
case 'X':
- DestWidth = 96;
+ DestWidth = 80;
break;
case 'T':
DestWidth = 128;
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -226,7 +226,7 @@
return Double;
switch (BitWidth) {
- case 96:
+ case 80:
if (() == ::APFloat::x87DoubleExtended)
return LongDouble;
break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
efriedma added inline comments.
Comment at: lib/Basic/TargetInfo.cpp:229
switch (BitWidth) {
- case 96:
+ case 80:
if (() == ::APFloat::x87DoubleExtended)
ddcc wrote:
> bruno wrote:
> > The change makes sense but I believe there's some historical reason why
> > this is 96 instead of 80? What problem have you found in practice? Do you
> > have a testcase or unittest that exercise the issue (and that could be
> > added to the patch)?
> I'd be curious why it was historically set to 96; I dug up rL190044 as the
> original commit, but it doesn't mention 80 vs 96 for this at all.
>
> I've been implementing an alternative symbolic constraint solver backend for
> the static analyzer, including floating-point support, which performs some
> type conversion and needs to reason about bitwidth. I'm still working on
> those patches, since they touch a lot of code and currently break some tests.
> I can write up a testcase for this issue, though I've only written IR
> testcases before and I'm not sure how I'd directly call a clang internal
> function?
It was set to 96 because that's what the only caller (handleModeAttr) expects;
see https://reviews.llvm.org/rL65935 and https://reviews.llvm.org/rL190926.
It's arbitrary as far as I know.
https://reviews.llvm.org/D26955
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
ddcc added inline comments.
Comment at: lib/Basic/TargetInfo.cpp:229
switch (BitWidth) {
- case 96:
+ case 80:
if (() == ::APFloat::x87DoubleExtended)
bruno wrote:
> The change makes sense but I believe there's some historical reason why this
> is 96 instead of 80? What problem have you found in practice? Do you have a
> testcase or unittest that exercise the issue (and that could be added to the
> patch)?
I'd be curious why it was historically set to 96; I dug up rL190044 as the
original commit, but it doesn't mention 80 vs 96 for this at all.
I've been implementing an alternative symbolic constraint solver backend for
the static analyzer, including floating-point support, which performs some type
conversion and needs to reason about bitwidth. I'm still working on those
patches, since they touch a lot of code and currently break some tests. I can
write up a testcase for this issue, though I've only written IR testcases
before and I'm not sure how I'd directly call a clang internal function?
https://reviews.llvm.org/D26955
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
bruno added inline comments.
Comment at: lib/Basic/TargetInfo.cpp:229
switch (BitWidth) {
- case 96:
+ case 80:
if (() == ::APFloat::x87DoubleExtended)
The change makes sense but I believe there's some historical reason why this is
96 instead of 80? What problem have you found in practice? Do you have a
testcase or unittest that exercise the issue (and that could be added to the
patch)?
https://reviews.llvm.org/D26955
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
ddcc added a comment. I could be completely mistaken here, but currently `Ctx.getRealTypeForBitwidth(llvm::APFloat::getSizeInBits(llvm::APFloat::x87DoubleExtended))` with a `ASTContext Ctx` does not round-trip correctly. https://reviews.llvm.org/D26955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26955: Fix bitwidth for x87 extended-precision floating-point type
ddcc created this revision.
ddcc added a reviewer: rsmith.
ddcc added a subscriber: cfe-commits.
llvm::APFloat::x87DoubleExtended is defined as having 80 bits of size
https://reviews.llvm.org/D26955
Files:
lib/Basic/TargetInfo.cpp
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -226,7 +226,7 @@
return Double;
switch (BitWidth) {
- case 96:
+ case 80:
if (() == ::APFloat::x87DoubleExtended)
return LongDouble;
break;
Index: lib/Basic/TargetInfo.cpp
===
--- lib/Basic/TargetInfo.cpp
+++ lib/Basic/TargetInfo.cpp
@@ -226,7 +226,7 @@
return Double;
switch (BitWidth) {
- case 96:
+ case 80:
if (() == ::APFloat::x87DoubleExtended)
return LongDouble;
break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
