[llvm] r372868 - Revert [IR] allow fast-math-flags on phi of FP values

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 25 06:29:09 PDT 2019


Author: spatel
Date: Wed Sep 25 06:29:09 2019
New Revision: 372868

URL: http://llvm.org/viewvc/llvm-project?rev=372868&view=rev
Log:
Revert [IR] allow fast-math-flags on phi of FP values

This reverts r372866 (git commit dec03223a97af0e4dfcb23da55c0f7f8c9b62d00)

Modified:
    llvm/trunk/docs/LangRef.rst
    llvm/trunk/include/llvm/IR/IRBuilder.h
    llvm/trunk/include/llvm/IR/Operator.h
    llvm/trunk/lib/Analysis/IVDescriptors.cpp
    llvm/trunk/lib/AsmParser/LLParser.cpp
    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
    llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
    llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
    llvm/trunk/test/Bitcode/compatibility.ll
    llvm/trunk/unittests/IR/InstructionsTest.cpp

Modified: llvm/trunk/docs/LangRef.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/LangRef.rst?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/docs/LangRef.rst (original)
+++ llvm/trunk/docs/LangRef.rst Wed Sep 25 06:29:09 2019
@@ -10067,7 +10067,7 @@ Syntax:
 
 ::
 
-      <result> = phi [fast-math-flags] <ty> [ <val0>, <label0>], ...
+      <result> = phi <ty> [ <val0>, <label0>], ...
 
 Overview:
 """""""""
@@ -10094,12 +10094,6 @@ deemed to occur on the edge from the cor
 the current block (but after any definition of an '``invoke``'
 instruction's return value on the same edge).
 
-The optional ``fast-math-flags`` marker indicates that the phi has one
-or more :ref:`fast-math-flags <fastmath>`. These are optimization hints
-to enable otherwise unsafe floating-point optimizations. Fast-math-flags
-are only valid for phis that return a floating-point scalar or vector
-type.
-
 Semantics:
 """"""""""
 

Modified: llvm/trunk/include/llvm/IR/IRBuilder.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/IRBuilder.h?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/IRBuilder.h (original)
+++ llvm/trunk/include/llvm/IR/IRBuilder.h Wed Sep 25 06:29:09 2019
@@ -2231,10 +2231,7 @@ public:
 
   PHINode *CreatePHI(Type *Ty, unsigned NumReservedValues,
                      const Twine &Name = "") {
-    PHINode *Phi = PHINode::Create(Ty, NumReservedValues);
-    if (isa<FPMathOperator>(Phi))
-      Phi = cast<PHINode>(setFPAttrs(Phi, nullptr /* MDNode* */, FMF));
-    return Insert(Phi, Name);
+    return Insert(PHINode::Create(Ty, NumReservedValues), Name);
   }
 
   CallInst *CreateCall(FunctionType *FTy, Value *Callee,

Modified: llvm/trunk/include/llvm/IR/Operator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Operator.h?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Operator.h (original)
+++ llvm/trunk/include/llvm/IR/Operator.h Wed Sep 25 06:29:09 2019
@@ -379,17 +379,13 @@ public:
       return false;
 
     switch (Opcode) {
-    // FIXME: To clean up and correct the semantics of fast-math-flags, FCmp
-    //        should not be treated as a math op, but the other opcodes should.
-    //        This would make things consistent with Select/PHI (FP value type
-    //        determines whether they are math ops and, therefore, capable of
-    //        having fast-math-flags).
     case Instruction::FCmp:
       return true;
     // non math FP Operators (no FMF)
     case Instruction::ExtractElement:
     case Instruction::ShuffleVector:
     case Instruction::InsertElement:
+    case Instruction::PHI:
       return false;
     default:
       return V->getType()->isFPOrFPVectorTy();

Modified: llvm/trunk/lib/Analysis/IVDescriptors.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/IVDescriptors.cpp?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/IVDescriptors.cpp (original)
+++ llvm/trunk/lib/Analysis/IVDescriptors.cpp Wed Sep 25 06:29:09 2019
@@ -300,8 +300,7 @@ bool RecurrenceDescriptor::AddReductionV
       ReduxDesc = isRecurrenceInstr(Cur, Kind, ReduxDesc, HasFunNoNaNAttr);
       if (!ReduxDesc.isRecurrence())
         return false;
-      // FIXME: FMF is allowed on phi, but propagation is not handled correctly.
-      if (isa<FPMathOperator>(ReduxDesc.getPatternInst()) && !IsAPhi)
+      if (isa<FPMathOperator>(ReduxDesc.getPatternInst()))
         FMF &= ReduxDesc.getPatternInst()->getFastMathFlags();
     }
 

Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
+++ llvm/trunk/lib/AsmParser/LLParser.cpp Wed Sep 25 06:29:09 2019
@@ -5802,19 +5802,7 @@ int LLParser::ParseInstruction(Instructi
   case lltok::kw_extractelement: return ParseExtractElement(Inst, PFS);
   case lltok::kw_insertelement:  return ParseInsertElement(Inst, PFS);
   case lltok::kw_shufflevector:  return ParseShuffleVector(Inst, PFS);
-  case lltok::kw_phi: {
-    FastMathFlags FMF = EatFastMathFlagsIfPresent();
-    int Res = ParsePHI(Inst, PFS);
-    if (Res != 0)
-      return Res;
-    if (FMF.any()) {
-      if (!Inst->getType()->isFPOrFPVectorTy())
-        return Error(Loc, "fast-math-flags specified for phi without "
-                          "floating-point scalar or vector return type");
-      Inst->setFastMathFlags(FMF);
-    }
-    return 0;
-  }
+  case lltok::kw_phi:            return ParsePHI(Inst, PFS);
   case lltok::kw_landingpad:     return ParseLandingPad(Inst, PFS);
   // Call.
   case lltok::kw_call:     return ParseCall(Inst, PFS, CallInst::TCK_None);

Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Wed Sep 25 06:29:09 2019
@@ -4629,48 +4629,31 @@ Error BitcodeReader::parseFunctionBody(F
       InstructionList.push_back(I);
       break;
     case bitc::FUNC_CODE_INST_PHI: { // PHI: [ty, val0,bb0, ...]
-      if (Record.size() < 1)
+      if (Record.size() < 1 || ((Record.size()-1)&1))
         return error("Invalid record");
-      // The first record specifies the type.
       FullTy = getFullyStructuredTypeByID(Record[0]);
       Type *Ty = flattenPointerTypes(FullTy);
       if (!Ty)
         return error("Invalid record");
 
-      // Phi arguments are pairs of records of [value, basic block].
-      // There is an optional final record for fast-math-flags if this phi has a
-      // floating-point type.
-      size_t NumArgs = (Record.size() - 1) / 2;
-      if ((Record.size() - 1) % 2 == 1 && !Ty->isFloatingPointTy())
-        return error("Invalid record");
-
-      PHINode *PN = PHINode::Create(Ty, NumArgs);
+      PHINode *PN = PHINode::Create(Ty, (Record.size()-1)/2);
       InstructionList.push_back(PN);
 
-      for (unsigned i = 0; i != NumArgs; i++) {
+      for (unsigned i = 0, e = Record.size()-1; i != e; i += 2) {
         Value *V;
         // With the new function encoding, it is possible that operands have
         // negative IDs (for forward references).  Use a signed VBR
         // representation to keep the encoding small.
         if (UseRelativeIDs)
-          V = getValueSigned(Record, i * 2 + 1, NextValueNo, Ty);
+          V = getValueSigned(Record, 1+i, NextValueNo, Ty);
         else
-          V = getValue(Record, i * 2 + 1, NextValueNo, Ty);
-        BasicBlock *BB = getBasicBlock(Record[i * 2 + 2]);
+          V = getValue(Record, 1+i, NextValueNo, Ty);
+        BasicBlock *BB = getBasicBlock(Record[2+i]);
         if (!V || !BB)
           return error("Invalid record");
         PN->addIncoming(V, BB);
       }
       I = PN;
-
-      // If there are an even number of records, the final record must be FMF.
-      if (Record.size() % 2 == 0) {
-        assert(isa<FPMathOperator>(I) && "Unexpected phi type");
-        FastMathFlags FMF = getDecodedFastMathFlags(Record[Record.size() - 1]);
-        if (FMF.any())
-          I->setFastMathFlags(FMF);
-      }
-
       break;
     }
 

Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
+++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Wed Sep 25 06:29:09 2019
@@ -2880,11 +2880,6 @@ void ModuleBitcodeWriter::writeInstructi
       pushValueSigned(PN.getIncomingValue(i), InstID, Vals64);
       Vals64.push_back(VE.getValueID(PN.getIncomingBlock(i)));
     }
-
-    uint64_t Flags = getOptimizationFlags(&I);
-    if (Flags != 0)
-      Vals64.push_back(Flags);
-
     // Emit a Vals64 vector and exit.
     Stream.EmitRecord(Code, Vals64, AbbrevToUse);
     Vals64.clear();

Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)
+++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Wed Sep 25 06:29:09 2019
@@ -5951,7 +5951,7 @@ bool CodeGenPrepare::optimizeSelectInst(
   // If branch conversion isn't desirable, exit early.
   if (DisableSelectToBranch || OptSize || !TLI)
     return false;
-TLI->isSelectSupported(<#SelectSupportKind#>)
+
   // Find all consecutive select instructions that share the same condition.
   SmallVector<SelectInst *, 2> ASI;
   ASI.push_back(SI);

Modified: llvm/trunk/test/Bitcode/compatibility.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/compatibility.ll?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/test/Bitcode/compatibility.ll (original)
+++ llvm/trunk/test/Bitcode/compatibility.ll Wed Sep 25 06:29:09 2019
@@ -861,27 +861,6 @@ define void @fastmathflags_vector_select
   ret void
 }
 
-define void @fastmathflags_phi(i1 %cond, float %f1, float %f2, double %d1, double %d2, half %h1, half %h2) {
-entry:
-  br i1 %cond, label %L1, label %L2
-L1:
-  br label %exit
-L2:
-  br label %exit
-exit:
-  %p.nnan = phi nnan float [ %f1, %L1 ], [ %f2, %L2 ]
-  ; CHECK: %p.nnan = phi nnan float [ %f1, %L1 ], [ %f2, %L2 ]
-  %p.ninf = phi ninf double [ %d1, %L1 ], [ %d2, %L2 ]
-  ; CHECK: %p.ninf = phi ninf double [ %d1, %L1 ], [ %d2, %L2 ]
-  %p.contract = phi contract half [ %h1, %L1 ], [ %h2, %L2 ]
-  ; CHECK: %p.contract = phi contract half [ %h1, %L1 ], [ %h2, %L2 ]
-  %p.nsz.reassoc = phi reassoc nsz float [ %f1, %L1 ], [ %f2, %L2 ]
-  ; CHECK: %p.nsz.reassoc = phi reassoc nsz float [ %f1, %L1 ], [ %f2, %L2 ]
-  %p.fast = phi fast half [ %h2, %L1 ], [ %h1, %L2 ]
-  ; CHECK: %p.fast = phi fast half [ %h2, %L1 ], [ %h1, %L2 ]
-  ret void
-}
-
 ; Check various fast math flags and floating-point types on calls.
 
 declare float @fmf1()

Modified: llvm/trunk/unittests/IR/InstructionsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/InstructionsTest.cpp?rev=372868&r1=372867&r2=372868&view=diff
==============================================================================
--- llvm/trunk/unittests/IR/InstructionsTest.cpp (original)
+++ llvm/trunk/unittests/IR/InstructionsTest.cpp Wed Sep 25 06:29:09 2019
@@ -1034,16 +1034,13 @@ TEST(InstructionsTest, SkipDebug) {
   EXPECT_EQ(nullptr, Term->getNextNonDebugInstruction());
 }
 
-TEST(InstructionsTest, PhiMightNotBeFPMathOperator) {
+TEST(InstructionsTest, PhiIsNotFPMathOperator) {
   LLVMContext Context;
   IRBuilder<> Builder(Context);
   MDBuilder MDHelper(Context);
-  Instruction *I = Builder.CreatePHI(Builder.getInt32Ty(), 0);
+  Instruction *I = Builder.CreatePHI(Builder.getDoubleTy(), 0);
   EXPECT_FALSE(isa<FPMathOperator>(I));
   I->deleteValue();
-  Instruction *FP = Builder.CreatePHI(Builder.getDoubleTy(), 0);
-  EXPECT_TRUE(isa<FPMathOperator>(FP));
-  FP->deleteValue();
 }
 
 TEST(InstructionsTest, FNegInstruction) {




More information about the llvm-commits mailing list