[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