[llvm] [Xtensa] Don't convert a select of FP constants into an indexed const… (PR #214974)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-xtensa

Author: Omid Safarzadeh (Bellman281)

<details>
<summary>Changes</summary>

`DAGCombiner::convertSelectOfFPConstantsToLoadOffset` rewrites `cond ? fc1 : fc2` into a load from a two-element constant pool array whose address is the pool base plus a condition-dependent offset.

Xtensa cannot select that. `XtensaTargetLowering::LowerConstantPool` returns a bare `XtensaISD::PCREL_WRAPPER`, and the only two ISel patterns consuming such a wrapper over a `tconstpool` require it to be the address operand of a load:

- `llvm/lib/Target/Xtensa/XtensaInstrInfo.td:239` — `[(set AR:$t, (load (Xtensa_pcrel_wrapper tconstpool:$label)))]`
- `llvm/lib/Target/Xtensa/XtensaInstrInfo.td:1313` — `def : Pat<(f32 (load (Xtensa_pcrel_wrapper tconstpool:$in))), (WFR (L32R tconstpool:$in))>;`

That is by design: `L32R` loads the *contents* of a PC-relative literal, and Xtensa has no instruction that materializes a literal's address. Once the combine inserts an `ISD::ADD` between the wrapper and the load, nothing matches and codegen aborts.

```llvm
; llc -mtriple=xtensa -mattr=+fp repro.ll
define float @<!-- -->select_fp_constants(float %x) nounwind {
  %cmp = fcmp olt float %x, 0.000000e+00
  %sel = select i1 %cmp, float -1.000000e+00, float 0.000000e+00
  ret float %sel
}
```

```
LLVM ERROR: Cannot select: i32 = XtensaISD::PCREL_WRAPPER
  TargetConstantPool:i32<[2 x float] [float 0.000000e+00, float -1.000000e+00]> 0
In function: select_fp_constants
```

Equivalently in C: `float f(float x) { return x < 0.0f ? -1.0f : 0.0f; }` at `-O2`.

Fixed by overriding `TargetLowering::reduceSelectOfFPConstantLoads`, the hook that exists for exactly this (X86 is the only other user today). Besides avoiding the crash, the transform would be a pessimization on Xtensa: the branch it replaces is cheaper than the extra address arithmetic and load.

Testing: both functions in the new test abort on unpatched `main` and produce correct code with the patch. `llvm/test/CodeGen/Xtensa` is 55/55 passing with it, unchanged from before. Verified against a from-source Release build of `main` at `5e30da9ace9b6667992673bb15c6dd0ed46b7b68`.

This is the narrow fix. The underlying hole is wider — `LowerConstantPool` returns a wrapper the backend can only select when a load consumes it directly, so any other path needing a constant pool *address* would fail the same way. The general fix would be to make `LowerConstantPool` mirror `LowerJumpTable`. That is a considerably larger change and this crash does not need it; happy to follow up if reviewers would prefer it.

Found while building a `no_std` Burn model for `xtensa-esp32s3-none-elf`; the crate-side workaround is tracel-ai/burn#<!-- -->5331.

---
Full diff: https://github.com/llvm/llvm-project/pull/214974.diff


1 Files Affected:

- (modified) llvm/lib/Target/Xtensa/XtensaISelLowering.h (+10) 


``````````diff
diff --git a/llvm/lib/Target/Xtensa/XtensaISelLowering.h b/llvm/lib/Target/Xtensa/XtensaISelLowering.h
index 0d455919cb407..0a7a603aae915 100644
--- a/llvm/lib/Target/Xtensa/XtensaISelLowering.h
+++ b/llvm/lib/Target/Xtensa/XtensaISelLowering.h
@@ -51,6 +51,16 @@ class XtensaTargetLowering : public TargetLowering {
 
   bool isFPImmLegal(const APFloat &Imm, EVT VT,
                     bool ForCodeSize) const override;
+  /// Xtensa reads constant pool entries with L32R, which loads the *contents*
+  /// of a PC-relative literal. There is no instruction that materializes the
+  /// address of a literal, so a constant pool base plus a variable offset
+  /// cannot be selected. Keep DAGCombiner from turning a select of two FP
+  /// constants into such an indexed constant pool load. Doing so would also be
+  /// a pessimization here: the branch it replaces is cheaper than the extra
+  /// address arithmetic and load.
+  bool reduceSelectOfFPConstantLoads(EVT CmpOpVT) const override {
+    return false;
+  }
 
   std::pair<unsigned, const TargetRegisterClass *>
   getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,

``````````

</details>


https://github.com/llvm/llvm-project/pull/214974