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&id=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. Perha
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 functi
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: llvm/li
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 thr
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 mu
10 matches
Mail list logo
