Lower OpFNegate directly to fneg (#2739)

Update SPIRVReader to lower OpFNegate directly to fneg. By doing this the code in SpirvLowerMath can be updated to remove the change to add a canonicalize for any fneg. The motivation for this is that InstCombine can introduce fneg instructions (after some recent upstream llvm changes) and in this case we don't want to blindly add a canonicalize as this invalidates e.g. xor 0x8000000 val -> fneg val. It makes more sense to handle the requirement to treat OpFNegate as a floating point instruction at the point it is lowered (llvm fneg is not, hence the reason we may need the canonicalize intrinsic).
GPUOpen-Drivers · Oct 3, 2023 · 1aebd12 · 1aebd12
1 parent 22cce3f
commit 1aebd12
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 14 deletions.
diff --git a/llpc/lower/llpcSpirvLowerMath.cpp b/llpc/lower/llpcSpirvLowerMath.cpp
@@ -475,15 +475,6 @@ bool SpirvLowerMathFloatOp::runImpl(Module &module) {
   return m_changed;
 }
 
-// =====================================================================================================================
-// Visits unary operator instruction.
-//
-// @param unaryOp : Unary operator instruction
-void SpirvLowerMathFloatOp::visitUnaryOperator(UnaryOperator &unaryOp) {
-  if (unaryOp.getOpcode() == Instruction::FNeg)
-    flushDenormIfNeeded(&unaryOp);
-}
-
 // =====================================================================================================================
 // Visits binary operator instruction.
 //

diff --git a/llpc/lower/llpcSpirvLowerMath.h b/llpc/lower/llpcSpirvLowerMath.h
@@ -98,7 +98,6 @@ class SpirvLowerMathFloatOp : public SpirvLowerMath,
   bool runImpl(llvm::Module &module);
 
   virtual void visitBinaryOperator(llvm::BinaryOperator &binaryOp);
-  virtual void visitUnaryOperator(llvm::UnaryOperator &unaryOp);
   virtual void visitCallInst(llvm::CallInst &callInst);
   virtual void visitFPTruncInst(llvm::FPTruncInst &fptruncInst);
 

diff --git a/llpc/test/shaderdb/core/OpFNegate_TestMat2X3_lit.frag b/llpc/test/shaderdb/core/OpFNegate_TestMat2X3_lit.frag
@@ -17,7 +17,7 @@ void main()
 /*
 ; RUN: amdllpc -v %gfxip %s | FileCheck -check-prefix=SHADERTEST %s
 ; SHADERTEST-LABEL: {{^// LLPC}} SPIRV-to-LLVM translation results
-; SHADERTEST-COUNT-2: fsub reassoc nnan nsz arcp contract afn <3 x float>
+; SHADERTEST-COUNT-2: fneg reassoc nnan nsz arcp contract afn <3 x float>
 ; SHADERTEST: AMDLLPC SUCCESS
 */
 // END_SHADERTEST
diff --git a/llpc/translator/lib/SPIRV/SPIRVReader.cpp b/llpc/translator/lib/SPIRV/SPIRVReader.cpp
@@ -5362,10 +5362,12 @@ Value *SPIRVToLLVM::transValueWithoutDecoration(SPIRVValue *bv, Function *f, Bas
   }
   case OpFNegate: {
     SPIRVUnary *bc = static_cast<SPIRVUnary *>(bv);
-    // Implement -x as -0.0 - x.
-    Value *negZero = ConstantFP::getNegativeZero(transType(bc->getType()));
-    auto fNeg = BinaryOperator::CreateFSub(negZero, transValue(bc->getOperand(0), f, bb), bv->getName(), bb);
+    Value *val0 = transValue(bc->getOperand(0), f, bb);
+    auto fNeg = getBuilder()->CreateFNeg(val0);
     setFastMathFlags(fNeg);
+    // Since OpFNegate is considered a floating point instruction, we must take
+    // that into account (since LLVM fneg is not).
+    fNeg = flushDenorm(fNeg);
     return mapValue(bv, fNeg);
   }