[llvm] [GlobalISel] Use correct fp semantics when building constants (PR #210190)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-globalisel

Author: nvmitch

<details>
<summary>Changes</summary>

This bug can materialize as a `bfloat 1.0` becoming `0x3C00` which is the bit pattern for `half 1.0`. After this change, `0x3F80` is correctly returned.
The fix is to use the destination type to convert the provided constant's semantics in `buildFConstant`. Added a unit test.
Removed `getAPFloatFromSize`, no more users.

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


4 Files Affected:

- (modified) llvm/include/llvm/CodeGen/GlobalISel/Utils.h (-3) 
- (modified) llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp (+5-3) 
- (modified) llvm/lib/CodeGen/GlobalISel/Utils.cpp (-13) 
- (modified) llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp (+18) 


``````````diff
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index 97d40ac1fdac3..2feaf76aaeaa6 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -299,9 +299,6 @@ T *getOpcodeDef(Register Reg, const MachineRegisterInfo &MRI) {
   return dyn_cast_or_null<T>(DefMI);
 }
 
-/// Returns an APFloat from Val converted to the appropriate size.
-LLVM_ABI APFloat getAPFloatFromSize(double Val, unsigned Size);
-
 /// Modify analysis usage so it preserves passes required for the SelectionDAG
 /// fallback.
 LLVM_ABI void getSelectionDAGFallbackAnalysisUsage(AnalysisUsage &AU);
diff --git a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
index bd6cd3c325951..d9eb4e7019be1 100644
--- a/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp
@@ -409,9 +409,11 @@ MachineInstrBuilder MachineIRBuilder::buildFConstant(const DstOp &Res,
                                                      double Val) {
   LLT DstTy = Res.getLLTTy(*getMRI());
   auto &Ctx = getMF().getFunction().getContext();
-  auto *CFP =
-      ConstantFP::get(Ctx, getAPFloatFromSize(Val, DstTy.getScalarSizeInBits()));
-  return buildFConstant(Res, *CFP);
+  APFloat APF(Val);
+  bool Ignored;
+  APF.convert(getFltSemanticForLLT(DstTy.getScalarType()),
+              APFloat::rmNearestTiesToEven, &Ignored);
+  return buildFConstant(Res, *ConstantFP::get(Ctx, APF));
 }
 
 MachineInstrBuilder MachineIRBuilder::buildFConstant(const DstOp &Res,
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index 7efde30bc88e2..52f2f089f7c46 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -659,19 +659,6 @@ MachineInstr *llvm::getOpcodeDef(unsigned Opcode, Register Reg,
   return DefMI && DefMI->getOpcode() == Opcode ? DefMI : nullptr;
 }
 
-APFloat llvm::getAPFloatFromSize(double Val, unsigned Size) {
-  if (Size == 32)
-    return APFloat(float(Val));
-  if (Size == 64)
-    return APFloat(Val);
-  if (Size != 16)
-    llvm_unreachable("Unsupported FPConstant size");
-  bool Ignored;
-  APFloat APF(Val);
-  APF.convert(APFloat::IEEEhalf(), APFloat::rmNearestTiesToEven, &Ignored);
-  return APF;
-}
-
 std::optional<APInt> llvm::ConstantFoldBinOp(unsigned Opcode,
                                              const Register Op1,
                                              const Register Op2,
diff --git a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
index 67e5c0964cba4..83fe8c7cb0f63 100644
--- a/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/MachineIRBuilderTest.cpp
@@ -37,6 +37,24 @@ TEST_F(AArch64GISelMITest, TestBuildConstantFConstant) {
   EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF;
 }
 
+TEST_F(AArch64GISelMITest, TestBuildFConstantBFloatSemantics) {
+  setUp();
+  if (!TM)
+    GTEST_SKIP();
+
+  // bfloat 1.0 -> 0x3F80
+  auto BF16One = B.buildFConstant(LLT::bfloat16(), 1.0);
+  const APFloat &BF16APF = BF16One->getOperand(1).getFPImm()->getValueAPF();
+  EXPECT_EQ(&BF16APF.getSemantics(), &APFloat::BFloat());
+  EXPECT_EQ(BF16APF.bitcastToAPInt().getZExtValue(), 0x3F80u);
+
+  // half 1.0 -> 0x3C00
+  auto F16One = B.buildFConstant(LLT::float16(), 1.0);
+  const APFloat &F16APF = F16One->getOperand(1).getFPImm()->getValueAPF();
+  EXPECT_EQ(&F16APF.getSemantics(), &APFloat::IEEEhalf());
+  EXPECT_EQ(F16APF.bitcastToAPInt().getZExtValue(), 0x3C00u);
+}
+
 #ifdef GTEST_HAS_DEATH_TEST
 #ifndef NDEBUG
 

``````````

</details>


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