[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
RalfJung wrote: > Generally clang-level warnings/errors are more user friendly then the > llvm-level errors (but both may be useful for other frontends). What would be really nice is if the LLVM backend could tell the frontend about the error rather than reporting it itself, that way frontends can use their nice error reporting code but don't have to re-implement the error detection logic. :) Anyway, that's a different issue... https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
RalfJung wrote: > I'm not yet convinced that llvm is the right place for this error message. > There's quite a lot of test changes that I presume needed to make and at > least the LTO use case looks like we don't want to require extra information. > I'd also prefer this to be a front-end error/warning as this is a > front-end/source-level mistake, with better diagnostic control options > available. There's a *lot* of complicated logic required to determine, for each architecture, which target features affect the ABI in which ways. Are you suggesting that logic should be duplicated across all frontends? That seems like a huge waste of effort to me. Most frontends are inevitably going to get it subtly wrong, so in the end we have a ton more bugs than we did if there was a central location where such know-how could be properly encoded, and where there are enough people that can confidently answer ABI questions like this. https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
RalfJung wrote: Yeah ideally LLVM would provide APIs where frontends can explicitly query that kind of information, and then provide nice errors with context. What this PR does is only the second-best think, in my eyes. But very few frontends can afford having an expert for each and every architecture LLVM supports, so "do it in the frontend" is often simply not an option. https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
RalfJung wrote: > converting llvm errors to clang errors. I assume you didn't also add those errors to all the other frontends, so every time you do this (assuming you actually remove the LLVM error at that point) you risk introducing bugs into other frontends (if they were relying on LLVM to reject some nonsensical situations). IMO this should be done with more consideration of non-clang frontends. https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
RalfJung wrote: As a Rust compiler dev, this part of LLVM feels extremely fragile. If we do anything wrong, LLVM will just use *some* ABI, and things will keep working for some situations, but really we have subtle ABI bugs that will eventually bite our users. The rules we have to follow are not documented anywhere (to my knowledge), and require target-specific expertise -- even if we get the x86 part right, we'll need a PowerPC expert to also get soft-float handling right for that target. It's not a pleasant experience at all. Having LLVM be more robust, and throw errors rather than being like "yeah let's just continue, I'm sure it's going to be fine", would go a long way towards making LLVM easier to use and more reliable as a backend. It is concerning if attempts at improving the robustness of LLVM in this space are being blocked. https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
RalfJung wrote: RISC-V has similar checks here: https://github.com/llvm/llvm-project/blob/ed572f2003275da8e06a634b4d6658b7921e8334/llvm/lib/Target/RISCV/RISCVISelLowering.cpp#L88-L100 So maybe the ARM checks could be added in a similar place? RISC-V handles ABI variants in a very clean way, and it does seem to work in practice too, so it'd be a good model for other architectures to follow. https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
https://github.com/RalfJung edited https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
https://github.com/RalfJung edited https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
@@ -311,11 +311,15 @@ ARMBaseTargetMachine::getSubtargetImpl(const Function &F)
const {
// function that reside in TargetOptions.
resetTargetOptions(F);
I = std::make_unique(TargetTriple, CPU, FS, *this, isLittle,
-F.hasMinSize());
+ F.hasMinSize());
if (!I->isThumb() && !I->hasARMOps())
F.getContext().emitError("Function '" + F.getName() + "' uses ARM "
"instructions, but the target does not support ARM mode execution.");
+
+if (I->isTargetHardFloat() && !I->hasFPRegs())
+ F.getContext().emitError("The hard-float ABI is enabled, but the target "
+ "lacks floating-point registers.");
RalfJung wrote:
This seems to check the target triple and do automatic hard float detection.
Shouldn't this instead check Options.FloatABIType?
https://github.com/llvm/llvm-project/pull/111334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
@@ -311,11 +311,15 @@ ARMBaseTargetMachine::getSubtargetImpl(const Function &F)
const {
// function that reside in TargetOptions.
resetTargetOptions(F);
I = std::make_unique(TargetTriple, CPU, FS, *this, isLittle,
-F.hasMinSize());
+ F.hasMinSize());
if (!I->isThumb() && !I->hasARMOps())
F.getContext().emitError("Function '" + F.getName() + "' uses ARM "
"instructions, but the target does not support ARM mode execution.");
+
+if (I->isTargetHardFloat() && !I->hasFPRegs())
+ F.getContext().emitError("The hard-float ABI is enabled, but the target "
+ "lacks floating-point registers.");
RalfJung wrote:
Yeah `ARMSubtarget.isTargetHardFloat` seems like a footgun, I hope the rest of
the backend properly has `Options.FloatABIType` override the target default.
https://github.com/llvm/llvm-project/pull/111334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
RalfJung wrote: @chrisnc what are the next steps here? Maybe one solution, for maximum compatibility, would be to not emit any warnings when `Options.FloatABIType` is left at `FloatABI::Default`, but if it is explicitly set to `FloatABI::Hard` and the required target features are missing, that should warn. (I assume `FloatABI::Soft` requires no target features.) `FloatABI::Hard` also seems to be incompatible with the `soft-float` target feature so that should probably also be a warning. https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM] Emit an error when the hard-float ABI is enabled but can't be used. (PR #111334)
RalfJung wrote: > there doesn't seem to be a mechanism to issue a warning from within LLVM > codegen, only to error. RISC-V does `errs() << "Hard-float 'f' ABI can't be used for a target that " ...` and that acts as a warning. It just prints a message to stderr. It's not pretty but it's better than generating code that has the wrong ABI and spending an hour debugging that... Rust will always set `Options.FloatABIType` to a non-`Default` value for ARM-32. https://github.com/llvm/llvm-project/pull/111334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
