[PATCH] D142953: [RISCV] Don't use constantpool for floating-point value if the value can be easily constructed by integer sequence and a floating-point move.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

I generally support this change once normal review completes.  I don't *think* this is likely to be too painful on any CPUs I'm currently aware of, but if it is, having it under a cpu tuning flag at a minimum makes sense.  I'd been thinking about this myself a bit in the background, and this patch looks basically identical to what I'd settled on heuristic wise.  This does kick in more than I'd expected.  Apparently, I'd misjudged how many floats this would actually pickup.



================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:732
+    switch (BitSize) {
+    case 16:
+      Opc = RISCV::FMV_H_X;
----------------
default: llvm_unreachable("message");


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1477
+                                              Subtarget.getFeatureBits());
+  // If the constantpool data is already in cache, only Cost 1 is cheaper.
+  return Cost <= FPImmCost;
----------------
I think this would be more clear if FPImmCost default to two (since the constant pool load uses two instructions), and you replace the <= with <.  Same result, but more obvious (to me).  


================
Comment at: llvm/test/CodeGen/RISCV/float-round-conv.ll:131
 ; RV32IF:       # %bb.0:
-; RV32IF-NEXT:    addi sp, sp, -16
-; RV32IF-NEXT:    .cfi_def_cfa_offset 16
-; RV32IF-NEXT:    sw ra, 12(sp) # 4-byte Folded Spill
-; RV32IF-NEXT:    .cfi_offset ra, -4
-; RV32IF-NEXT:    lui a0, %hi(.LCPI7_0)
-; RV32IF-NEXT:    flw ft0, %lo(.LCPI7_0)(a0)
+; RV32IF-NEXT:    lui a0, 307200
+; RV32IF-NEXT:    fmv.w.x ft0, a0
----------------
Not directly related to this patch, but these test diffs make it look like a constant pool load is blocking stack setup/tear down optimization.  (Sorry, there's a particular name for this optimization, but my brain is blanking right now.)  We should probably fix that separately.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142953/new/

https://reviews.llvm.org/D142953