[IR] Add support for `nneg` flag with `uitofp` #86141

goldsteinn · 2024-03-21T15:54:41Z

As noted when #82404 was pushed (canonicalizing sitofp -> uitofp),
different signedness on fp casts can have dramatic performance
implications on different backends.

So, it makes to create a reliable means for the backend to pick its
cast signedness if either are correct.

Further, this allows us to start canonicalizing sitofp- > uitofp
which may easy middle end analysis.

llvmbot · 2024-03-21T15:55:11Z

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-llvm-selectiondag
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: None (goldsteinn)

Changes

As noted when #82404 was pushed (canonicalizing sitofp -> uitofp),
different signedness on fp casts can have dramatic performance
implications on different backends.

So, it makes to create a reliable means for the backend to pick its
cast signedness if either are correct.

Further, this allows us to start canonicalizing sitofp- > uitofp
which may easy middle end analysis.

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

11 Files Affected:

(modified) llvm/docs/LangRef.rst (+10)
(modified) llvm/include/llvm/CodeGen/TargetLowering.h (+6)
(modified) llvm/include/llvm/IR/InstrTypes.h (+8-2)
(modified) llvm/lib/AsmParser/LLParser.cpp (+1-1)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (+9-1)
(modified) llvm/lib/IR/Instruction.cpp (+3-2)
(modified) llvm/lib/IR/Operator.cpp (+1)
(modified) llvm/test/Assembler/flags.ll (+7)
(modified) llvm/test/Bitcode/flags.ll (+4)
(modified) llvm/test/Transforms/InstCombine/freeze.ll (+11)
(modified) llvm/test/Transforms/SimplifyCFG/HoistCode.ll (+31)

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 8bc1cab01bf0a6..08da14bf86c054 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -11616,6 +11616,10 @@ Overview:
 The '``uitofp``' instruction regards ``value`` as an unsigned integer
 and converts that value to the ``ty2`` type.
 
+The ``nneg`` (non-negative) flag, if present, specifies that the
+operand is non-negative. This property may be used by optimization
+passes to later convert the ``uitofp`` into a ``sitofp``.
+
 Arguments:
 """"""""""
 
@@ -11633,6 +11637,9 @@ integer quantity and converts it to the corresponding floating-point
 value. If the value cannot be exactly represented, it is rounded using
 the default rounding mode.
 
+If the ``nneg`` flag is set, and the ``uitofp`` argument is negative,
+the result is a poison value.
+
 
 Example:
 """"""""
@@ -11642,6 +11649,9 @@ Example:
       %X = uitofp i32 257 to float         ; yields float:257.0
       %Y = uitofp i8 -1 to double          ; yields double:255.0
 
+      %a = uitofp nneg i32 256 to i32      ; yields float:257.0
+      %b = uitofp nneg i32 -256 to i32     ; yields i32 poison
+
 '``sitofp .. to``' Instruction
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 59fad88f91b1d1..c53de1d4b6d61e 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -3005,6 +3005,12 @@ class TargetLoweringBase {
     return false;
   }
 
+  /// Return true if sitofp from FromTy to ToTy is cheaper than
+  /// uitofp.
+  virtual bool isSIToFPCheaperThanUIToFP(EVT FromTy, EVT ToTy) const {
+    return false;
+  }
+
   /// Return true if this constant should be sign extended when promoting to
   /// a larger type.
   virtual bool signExtendConstant(const ConstantInt *C) const { return false; }
diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index e8c2cba8418dc8..8e2eff2e65247d 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -933,13 +933,19 @@ class CastInst : public UnaryInstruction {
   }
 };
 
-/// Instruction that can have a nneg flag (only zext).
+/// Instruction that can have a nneg flag (zext/uitofp).
 class PossiblyNonNegInst : public CastInst {
 public:
   enum { NonNeg = (1 << 0) };
 
   static bool classof(const Instruction *I) {
-    return I->getOpcode() == Instruction::ZExt;
+    switch (I->getOpcode()) {
+    case Instruction::ZExt:
+    case Instruction::UIToFP:
+      return true;
+    default:
+      return false;
+    }
   }
 
   static bool classof(const Value *V) {
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
index f0be021668afa7..ca3973900ff969 100644
--- a/llvm/lib/AsmParser/LLParser.cpp
+++ b/llvm/lib/AsmParser/LLParser.cpp
@@ -6801,6 +6801,7 @@ int LLParser::parseInstruction(Instruction *&Inst, BasicBlock *BB,
   }
 
   // Casts.
+  case lltok::kw_uitofp:
   case lltok::kw_zext: {
     bool NonNeg = EatIfPresent(lltok::kw_nneg);
     bool Res = parseCast(Inst, PFS, KeywordVal);
@@ -6816,7 +6817,6 @@ int LLParser::parseInstruction(Instruction *&Inst, BasicBlock *BB,
   case lltok::kw_fpext:
   case lltok::kw_bitcast:
   case lltok::kw_addrspacecast:
-  case lltok::kw_uitofp:
   case lltok::kw_sitofp:
   case lltok::kw_fptoui:
   case lltok::kw_fptosi:
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 2d63774c75e372..010c5c6c48e40a 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -3882,7 +3882,15 @@ void SelectionDAGBuilder::visitUIToFP(const User &I) {
   SDValue N = getValue(I.getOperand(0));
   EVT DestVT = DAG.getTargetLoweringInfo().getValueType(DAG.getDataLayout(),
                                                         I.getType());
-  setValue(&I, DAG.getNode(ISD::UINT_TO_FP, getCurSDLoc(), DestVT, N));
+  bool IsNonNeg = false;
+  if (auto *PNI = dyn_cast<PossiblyNonNegInst>(&I))
+    IsNonNeg = true;
+
+  unsigned Opc = ISD::UINT_TO_FP;
+  if (IsNonNeg && DAG.getTargetLoweringInfo().isSIToFPCheaperThanUIToFP(
+                      N.getValueType(), DestVT))
+    Opc = ISD::SINT_TO_FP;
+  setValue(&I, DAG.getNode(Opc, getCurSDLoc(), DestVT, N));
 }
 
 void SelectionDAGBuilder::visitSIToFP(const User &I) {
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 47a7f2c9de790f..7f11ffacf26501 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -382,7 +382,7 @@ void Instruction::setIsExact(bool b) {
 }
 
 void Instruction::setNonNeg(bool b) {
-  assert(isa<PossiblyNonNegInst>(this) && "Must be zext");
+  assert(isa<PossiblyNonNegInst>(this) && "Must be zext/uitofp");
   SubclassOptionalData = (SubclassOptionalData & ~PossiblyNonNegInst::NonNeg) |
                          (b * PossiblyNonNegInst::NonNeg);
 }
@@ -396,7 +396,7 @@ bool Instruction::hasNoSignedWrap() const {
 }
 
 bool Instruction::hasNonNeg() const {
-  assert(isa<PossiblyNonNegInst>(this) && "Must be zext");
+  assert(isa<PossiblyNonNegInst>(this) && "Must be zext/uitofp");
   return (SubclassOptionalData & PossiblyNonNegInst::NonNeg) != 0;
 }
 
@@ -429,6 +429,7 @@ void Instruction::dropPoisonGeneratingFlags() {
     cast<GetElementPtrInst>(this)->setIsInBounds(false);
     break;
 
+  case Instruction::UIToFP:
   case Instruction::ZExt:
     setNonNeg(false);
     break;
diff --git a/llvm/lib/IR/Operator.cpp b/llvm/lib/IR/Operator.cpp
index b9cd219d94dc8a..6603ac36239096 100644
--- a/llvm/lib/IR/Operator.cpp
+++ b/llvm/lib/IR/Operator.cpp
@@ -39,6 +39,7 @@ bool Operator::hasPoisonGeneratingFlags() const {
     // Note: inrange exists on constexpr only
     return GEP->isInBounds() || GEP->getInRange() != std::nullopt;
   }
+  case Instruction::UIToFP:
   case Instruction::ZExt:
     if (auto *NNI = dyn_cast<PossiblyNonNegInst>(this))
       return NNI->hasNonNeg();
diff --git a/llvm/test/Assembler/flags.ll b/llvm/test/Assembler/flags.ll
index 04bddd02f50c81..c4f1d4c288b8d5 100644
--- a/llvm/test/Assembler/flags.ll
+++ b/llvm/test/Assembler/flags.ll
@@ -256,6 +256,13 @@ define i64 @test_zext(i32 %a) {
   ret i64 %res
 }
 
+define float @test_uitofp(i32 %a) {
+; CHECK: %res = uitofp nneg i32 %a to float
+  %res = uitofp nneg i32 %a to float
+  ret float %res
+}
+
+
 define i64 @test_or(i64 %a, i64 %b) {
 ; CHECK: %res = or disjoint i64 %a, %b
   %res = or disjoint i64 %a, %b
diff --git a/llvm/test/Bitcode/flags.ll b/llvm/test/Bitcode/flags.ll
index e3fc827d865d7e..5d41e441b5ced4 100644
--- a/llvm/test/Bitcode/flags.ll
+++ b/llvm/test/Bitcode/flags.ll
@@ -18,6 +18,8 @@ second:                                           ; preds = %first
   %z = add i32 %a, 0                              ; <i32> [#uses=0]
   %hh = zext nneg i32 %a to i64
   %ll = zext i32 %s to i64
+  %ff = uitofp nneg i32 %a to float
+  %bb = uitofp i32 %s to float
   %jj = or disjoint i32 %a, 0
   %oo = or i32 %a, 0
   unreachable
@@ -30,6 +32,8 @@ first:                                            ; preds = %entry
   %zz = add i32 %a, 0                             ; <i32> [#uses=0]
   %kk = zext nneg i32 %a to i64
   %rr = zext i32 %ss to i64
+  %ww = uitofp nneg i32 %a to float
+  %xx = uitofp i32 %ss to float
   %mm = or disjoint i32 %a, 0
   %nn = or i32 %a, 0
   br label %second
diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll
index da59101d5710cb..2342184f8221e6 100644
--- a/llvm/test/Transforms/InstCombine/freeze.ll
+++ b/llvm/test/Transforms/InstCombine/freeze.ll
@@ -1127,6 +1127,17 @@ define i32 @freeze_zext_nneg(i8 %x) {
   ret i32 %fr
 }
 
+define float @freeze_uitofp_nneg(i8 %x) {
+; CHECK-LABEL: @freeze_uitofp_nneg(
+; CHECK-NEXT:    [[X_FR:%.*]] = freeze i8 [[X:%.*]]
+; CHECK-NEXT:    [[UITOFP:%.*]] = uitofp i8 [[X_FR]] to float
+; CHECK-NEXT:    ret float [[UITOFP]]
+;
+  %uitofp = uitofp nneg i8 %x to float
+  %fr = freeze float %uitofp
+  ret float %fr
+}
+
 define i32 @propagate_drop_flags_or(i32 %arg) {
 ; CHECK-LABEL: @propagate_drop_flags_or(
 ; CHECK-NEXT:    [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]]
diff --git a/llvm/test/Transforms/SimplifyCFG/HoistCode.ll b/llvm/test/Transforms/SimplifyCFG/HoistCode.ll
index a081eddfc45660..89a13cead35e06 100644
--- a/llvm/test/Transforms/SimplifyCFG/HoistCode.ll
+++ b/llvm/test/Transforms/SimplifyCFG/HoistCode.ll
@@ -125,6 +125,37 @@ F:
   ret i32 %z2
 }
 
+
+define float @hoist_uitofp_flags_preserve(i1 %C, i8 %x) {
+; CHECK-LABEL: @hoist_uitofp_flags_preserve(
+; CHECK-NEXT:  common.ret:
+; CHECK-NEXT:    [[Z1:%.*]] = uitofp nneg i8 [[X:%.*]] to float
+; CHECK-NEXT:    ret float [[Z1]]
+;
+  br i1 %C, label %T, label %F
+T:
+  %z1 = uitofp nneg i8 %x to float
+  ret float %z1
+F:
+  %z2 = uitofp nneg i8 %x to float
+  ret float %z2
+}
+
+define float @hoist_uitofp_flags_drop(i1 %C, i8 %x) {
+; CHECK-LABEL: @hoist_uitofp_flags_drop(
+; CHECK-NEXT:  common.ret:
+; CHECK-NEXT:    [[Z1:%.*]] = uitofp i8 [[X:%.*]] to float
+; CHECK-NEXT:    ret float [[Z1]]
+;
+  br i1 %C, label %T, label %F
+T:
+  %z1 = uitofp nneg i8 %x to float
+  ret float %z1
+F:
+  %z2 = uitofp i8 %x to float
+  ret float %z2
+}
+
 define i32 @hoist_or_flags_preserve(i1 %C, i32 %x, i32 %y) {
 ; CHECK-LABEL: @hoist_or_flags_preserve(
 ; CHECK-NEXT:  common.ret:

llvm/include/llvm/CodeGen/TargetLowering.h

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

github-actions · 2024-03-21T20:19:18Z

✅ With the latest revision this PR passed the C/C++ code formatter.

tschuett · 2024-03-22T06:33:09Z

MachineInstr and MachineInstr::copyFlagsFromInstruction will eventually need to support the nneg flag.

goldsteinn · 2024-03-22T06:36:26Z

MachineInstr and MachineInstr::copyFlagsFromInstruction will eventually need to support the nneg flag.

Surprised that wasn't added in when zext nneg support was added. @topperc any reason?

llvm/lib/Target/X86/X86ISelLowering.cpp

arsenm

I think the IR and codegen changes should be split to separate patches. Also missing the GlobalIsel handling

tschuett · 2024-03-26T08:54:58Z

While at it, disjoint is also missing in GlobalIsel.

goldsteinn · 2024-03-26T17:06:21Z

I think the IR and codegen changes should be split to separate patches. Also missing the GlobalIsel handling

Okay, this change is IR only now.

goldsteinn · 2024-03-26T17:31:42Z

I think the IR and codegen changes should be split to separate patches. Also missing the GlobalIsel handling

Okay, this change is IR only now.

Realize still have logic in SelectionDAGBuilder. Do you want me to split that as well?

arsenm · 2024-03-27T09:17:18Z

Realize still have logic in SelectionDAGBuilder. Do you want me to split that as well?

That's the codegen part, so yes?

nikic · 2024-03-27T12:14:12Z

This should probably have a small RFC on discourse.

goldsteinn · 2024-03-27T18:08:40Z

Realize still have logic in SelectionDAGBuilder. Do you want me to split that as well?

That's the codegen part, so yes?

Done

goldsteinn · 2024-03-27T18:34:28Z

This should probably have a small RFC on discourse.

Done, see: https://discourse.llvm.org/t/rfc-support-nneg-flag-with-uitofp/77988
which is basically a copy and paste of your trunc nw post.

dtcxzyw

Please update BitcodeReader.cpp as well.

llvm-project/llvm/lib/Bitcode/Reader/BitcodeReader.cpp

Lines 5027 to 5029 in e69cab7

    
           if (Opc == Instruction::ZExt) { 
        
             if (Record[OpNum] & (1 << bitc::PNNI_NON_NEG)) 
        
               cast<PossiblyNonNegInst>(I)->setNonNeg(true);

llvm/docs/LangRef.rst

llvm/include/llvm/IR/IRBuilder.h

dtcxzyw

LGTM. Thanks!

nikic

LGTM

nikic · 2024-04-08T05:13:59Z

llvm/include/llvm/IR/IRBuilder.h

@@ -2143,12 +2137,15 @@ class IRBuilderBase {
  }

  Value *CreateCast(Instruction::CastOps Op, Value *V, Type *DestTy,
-                    const Twine &Name = "") {
+                    const Twine &Name = "", bool IsNonNeg = false) {


I think I'd rather not have this, given how the flag does not apply to all casts.

ill revert that.

As noted when llvm#82404 was pushed (canonicalizing `sitofp` -> `uitofp`), different signedness on fp casts can have dramatic performance implications on different backends. So, it makes to create a reliable means for the backend to pick its cast signedness if either are correct. Further, this allows us to start canonicalizing `sitofp`- > `uitofp` which may easy middle end analysis.

llvmbot added llvm:SelectionDAG SelectionDAGISel as well llvm:ir llvm:transforms labels Mar 21, 2024

goldsteinn mentioned this pull request Mar 21, 2024

[InstCombine] Canonicalize (sitofp x) -> (uitofp x) if x >= 0 #82404

Closed

goldsteinn requested review from nikic, arsenm, OCHyams, alexfh, dtcxzyw, topperc and asmok-g March 21, 2024 15:55

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from 7c57411 to bb7e56c Compare March 21, 2024 16:02

goldsteinn mentioned this pull request Mar 21, 2024

[CVP][SCCP] Add support for uitofp nneg #86154

Closed

dtcxzyw reviewed Mar 21, 2024

View reviewed changes

llvm/include/llvm/CodeGen/TargetLowering.h Outdated Show resolved Hide resolved

tschuett reviewed Mar 21, 2024

View reviewed changes

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Outdated Show resolved Hide resolved

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from bb7e56c to 156943d Compare March 21, 2024 20:16

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from 156943d to c22abc1 Compare March 21, 2024 20:27

llvmbot added the backend:X86 label Mar 21, 2024

goldsteinn requested review from RKSimon and phoebewang March 21, 2024 20:43

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from aa94714 to 8796f55 Compare March 22, 2024 16:52

RKSimon reviewed Mar 22, 2024

View reviewed changes

llvm/lib/Target/X86/X86ISelLowering.cpp Outdated Show resolved Hide resolved

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from 8796f55 to 0142db5 Compare March 22, 2024 17:31

arsenm reviewed Mar 26, 2024

View reviewed changes

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from 0142db5 to 8547c43 Compare March 26, 2024 17:06

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from 8547c43 to 1d9e1c1 Compare March 27, 2024 18:08

goldsteinn changed the title ~~[IR][DAG] Add support for nneg flag with uitofp~~ [IR] Add support for nneg flag with uitofp Mar 27, 2024

dtcxzyw requested changes Apr 4, 2024

View reviewed changes

llvm/docs/LangRef.rst Outdated Show resolved Hide resolved

llvm/include/llvm/IR/IRBuilder.h Outdated Show resolved Hide resolved

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from 1d9e1c1 to 8aba264 Compare April 4, 2024 18:31

dtcxzyw approved these changes Apr 4, 2024

View reviewed changes

nikic approved these changes Apr 8, 2024

View reviewed changes

goldsteinn force-pushed the goldsteinn/uito-fp-support branch from 8aba264 to 08f1fe9 Compare April 9, 2024 05:30

goldsteinn closed this in 9170e38 Apr 9, 2024

dtcxzyw mentioned this pull request Apr 17, 2024

Add support for uitofp nneg AliveToolkit/alive2#1027

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IR] Add support for `nneg` flag with `uitofp` #86141

[IR] Add support for `nneg` flag with `uitofp` #86141

goldsteinn commented Mar 21, 2024 •

edited

llvmbot commented Mar 21, 2024 •

edited

github-actions bot commented Mar 21, 2024 •

edited

tschuett commented Mar 22, 2024

goldsteinn commented Mar 22, 2024

arsenm left a comment

tschuett commented Mar 26, 2024

goldsteinn commented Mar 26, 2024

goldsteinn commented Mar 26, 2024

arsenm commented Mar 27, 2024

nikic commented Mar 27, 2024

goldsteinn commented Mar 27, 2024

goldsteinn commented Mar 27, 2024

dtcxzyw left a comment

dtcxzyw left a comment

nikic left a comment

nikic Apr 8, 2024

goldsteinn Apr 9, 2024

	if (Opc == Instruction::ZExt) {
	if (Record[OpNum] & (1 << bitc::PNNI_NON_NEG))
	cast<PossiblyNonNegInst>(I)->setNonNeg(true);

[IR] Add support for nneg flag with uitofp #86141

[IR] Add support for nneg flag with uitofp #86141

Conversation

goldsteinn commented Mar 21, 2024 • edited

llvmbot commented Mar 21, 2024 • edited

github-actions bot commented Mar 21, 2024 • edited

tschuett commented Mar 22, 2024

goldsteinn commented Mar 22, 2024

arsenm left a comment

Choose a reason for hiding this comment

tschuett commented Mar 26, 2024

goldsteinn commented Mar 26, 2024

goldsteinn commented Mar 26, 2024

arsenm commented Mar 27, 2024

nikic commented Mar 27, 2024

goldsteinn commented Mar 27, 2024

goldsteinn commented Mar 27, 2024

dtcxzyw left a comment

Choose a reason for hiding this comment

dtcxzyw left a comment

Choose a reason for hiding this comment

nikic left a comment

Choose a reason for hiding this comment

nikic Apr 8, 2024

Choose a reason for hiding this comment

goldsteinn Apr 9, 2024

Choose a reason for hiding this comment

[IR] Add support for `nneg` flag with `uitofp` #86141

[IR] Add support for `nneg` flag with `uitofp` #86141

goldsteinn commented Mar 21, 2024 •

edited

llvmbot commented Mar 21, 2024 •

edited

github-actions bot commented Mar 21, 2024 •

edited