simon_tatham marked an inline comment as done.
simon_tatham added a comment.
Hmm, yes, I see what you mean. It //looks// to me as if the problem is that
`llvm::ARM::getFPUFeatures` is setting up a feature list including `+vfp4`,
`-fp64` and `-d32` just as you'd expect, but when it's called from
JamesNagurne added a comment.
Hi Simon et. al., I'm working on a downstream ARM toolchain and have
downstreamed this change into our codebase.
We saw that you've fixed the -mfpu=none issue and have taken that as well, but
are still running into some issues.
Prior to your change, the optionset
This revision was automatically updated to reflect the committed changes.
Closed by commit rC361845: [ARM] Replace fp-only-sp and d16 with fp64 and d32.
(authored by statham, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D60691?vs=201658=201691#toc
Repository:
rC Clang
SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.
There already was consensus that this is was a good change, and also that we
don't care about auto-upgrading. With the last minor comments addressed, this
looks good I think.
simon_tatham marked 4 inline comments as done.
simon_tatham added inline comments.
Comment at: llvm/lib/Target/ARM/ARMSubtarget.h:587
bool hasVFP2() const { return HasVFPv2; }
bool hasVFP3() const { return HasVFPv3; }
ostannard wrote:
> Are the old
simon_tatham marked an inline comment as done.
simon_tatham added inline comments.
Comment at: llvm/test/MC/ARM/armv8.3a-js.s:16
// REQ-V83: error: instruction requires: armv8.3a
-// REQ-FP: error: instruction requires: FPARMv8
+// REQ-FP: error: invalid instruction
ostannard added inline comments.
Comment at: clang/test/Driver/arm-mfpu.c:112
+// CHECK-VFP3XD-NOT: "-target-feature" "+fp64"
+// CHECK-VFP3XD-NOT: "-target-feature" "+32"
// CHECK-VFP3XD: "-target-feature" "+vfp3"
"+d32" ?
Comment at:
dmgreen added a comment.
I don't think we here care about auto-updating, not supporting bitcode/lto
libraries.
Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:1362
+ if (Ty->isDoubleTy() && (!Subtarget->hasVFP2Base() || !Subtarget->hasFP64()))
+ return false;
ostannard edited reviewers, added: ostannard; removed: olista01.
ostannard added a comment.
For the auto-upgrader, these limited FPUs only exist for microcontrollers,
where I doubt any users are keeping bitcode files around for a long time, so
I'd be fine with not doing this. I've had a skim
t.p.northover added a comment.
I like the direction of this change, and the details look correct too.
The one thing I wonder about is whether we should be upgrading .bc files too
(or otherwise support fp-only-sp in legacy inputs). I think it's a specialized
enough feature that there won't be
10 matches
Mail list logo
