[llvm] [llvm] Ensure that soft float targets don't use float/vector code for memops. (PR #107022)
Alex Rønne Petersen via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 4 08:56:34 PDT 2024
================
@@ -8846,9 +8846,14 @@ static bool findGISelOptimalMemOpLowering(std::vector<LLT> &MemOps,
if (Op.isMemcpyWithFixedDstAlign() && Op.getSrcAlign() < Op.getDstAlign())
return false;
- LLT Ty = TLI.getOptimalMemOpLLT(Op, FuncAttributes);
-
- if (Ty == LLT()) {
+ bool WantIntScalar = TLI.useSoftFloat() ||
+ FuncAttributes.hasFnAttr(Attribute::NoImplicitFloat);
----------------
alexrp wrote:
> I'd prefer to avoid eagerly querying these edge cases.
Do you mean that you would prefer if I just fixed each `getOptimalMemOpLLT()`/`getOptimalMemOpType()` implementation to check for these attributes when they want to potentially return a float/vector type?
> Also this seems like another case where we have overlapping and poorly defined mechanisms. For example useSoftFloat is a virtual function without any arguments, which is kind of useless. Why isn't it a simple function field, and isn't it a function of the use-soft-float attribute, so it can't just be part of the subtarget's TargetLowering?
The way soft float is handled in function attributes and subtarget features is a bit confusing, and it's not helped by the fact that some targets do it slightly differently.
As I understand it, the *general* idea is that if you set the `use-soft-float` attribute on a function, this should result in the `+soft-float` (or sometimes `-hard-float`) subtarget feature being enabled for that function. The idea is then that the `TargetLowering::useSoftFloat()` override can check the target-specific `soft-float`/`hard-float` feature on its subtarget object.
Of course, it *seems* like all of this could be significantly simplified by not going through that intermediate `soft-float`/`hard-float` step and just making `use-soft-float` the source of truth. I might try to clean this up in a future patch, but I didn't want to rock the boat too much in this PR.
> At least could just merge these into a helper for implementations of getOptimalMemOpLLT to use?
Agree, that seems reasonable.
https://github.com/llvm/llvm-project/pull/107022
More information about the llvm-commits
mailing list