Address review comments

- Add IRBuilder::resetModeToStrictFP instead of adding extra argument to
  setIsFPConstrained.
- Change wording in the Release Notes.
- Modify printing MemoryEffects of FP operation.
- Fix IRBuilder::CreateCall.
- Clean up unit test.

Author: spavloff

clang/lib/CodeGen/CodeGenFunction.cpp

@@ -1092,7 +1092,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
   if ((FD && (FD->UsesFPIntrin() || FD->hasAttr<StrictFPAttr>())) ||
       (!FD && (FPExceptionBehavior != llvm::fp::ebIgnore ||
                RM != llvm::RoundingMode::NearestTiesToEven))) {
-    Builder.setIsFPConstrained(true, false);
+    Builder.setIsFPConstrained(true);
     Fn->addFnAttr(llvm::Attribute::StrictFP);
   }
 

llvm/docs/ReleaseNotes.md

@@ -56,7 +56,8 @@ Makes programs 10x faster by doing Special New Thing.
 Changes to the LLVM IR
 ----------------------
 
-* Floating-point operand bundles have been added.
+* Calls to floating-point intrinsics can have operand bundles "fp.round" and
+  "fp.except", which specify effective rounding mode and exception behavior.
 
 Changes to LLVM infrastructure
 ------------------------------

llvm/include/llvm/IR/FPEnv.h

@@ -81,6 +81,11 @@ LLVM_ABI std::optional<StringRef>
 std::optional<StringRef>
     convertExceptionBehaviorToBundle(fp::ExceptionBehavior);
 
+inline raw_ostream &operator<<(raw_ostream &OS, fp::ExceptionBehavior EB) {
+  OS << convertExceptionBehaviorToBundle(EB).value_or("invalid");
+  return OS;
+}
+
 /// Returns true if the exception handling behavior and rounding mode
 /// match what is used in the default floating point environment.
 inline bool isDefaultFPEnvironment(fp::ExceptionBehavior EB, RoundingMode RM) {

llvm/include/llvm/IR/IRBuilder.h

@@ -348,17 +348,19 @@ class IRBuilderBase {
   /// enabled the CreateF<op>() calls instead create constrained
   /// floating point intrinsic calls. Fast math flags are unaffected
   /// by this setting.
-  void setIsFPConstrained(bool IsCon, bool AndReset = true) {
-    if (AndReset) {
-      if (IsCon) {
-        setDefaultConstrainedRounding(RoundingMode::Dynamic);
-        setDefaultConstrainedExcept(fp::ebStrict);
-      } else {
-        setDefaultConstrainedRounding(RoundingMode::NearestTiesToEven);
-        setDefaultConstrainedExcept(fp::ebIgnore);
-      }
+  void setIsFPConstrained(bool IsCon) { IsFPConstrained = IsCon; }
+
+  /// Enable/Disable use of constrained floating point math and reset FP options
+  /// according to the selected mode.
+  void resetModeToStrictFP(bool IsCon) {
+    if (IsCon) {
+      setDefaultConstrainedRounding(RoundingMode::Dynamic);
+      setDefaultConstrainedExcept(fp::ebStrict);
+    } else {
+      setDefaultConstrainedRounding(RoundingMode::NearestTiesToEven);
+      setDefaultConstrainedExcept(fp::ebIgnore);
     }
-    IsFPConstrained = IsCon;
+    setIsFPConstrained(IsCon);
   }
 
   /// Query for the use of constrained floating point math

llvm/lib/CodeGen/AtomicExpandPass.cpp

@@ -159,7 +159,7 @@ struct ReplacementIRBuilder
     SetInsertPoint(I);
     this->CollectMetadataToCopy(I, {LLVMContext::MD_pcsections});
     if (BB->getParent()->getAttributes().hasFnAttr(Attribute::StrictFP))
-      this->setIsFPConstrained(true);
+      this->resetModeToStrictFP(true);
 
     MMRAMD = I->getMetadata(LLVMContext::MD_mmra);
   }
@@ -1706,7 +1706,7 @@ bool AtomicExpandImpl::tryExpandAtomicCmpXchg(AtomicCmpXchgInst *CI) {
 bool llvm::expandAtomicRMWToCmpXchg(AtomicRMWInst *AI,
                                     CreateCmpXchgInstFun CreateCmpXchg) {
   ReplacementIRBuilder Builder(AI, AI->getDataLayout());
-  Builder.setIsFPConstrained(
+  Builder.resetModeToStrictFP(
       AI->getFunction()->hasFnAttribute(Attribute::StrictFP));
 
   // FIXME: If FP exceptions are observable, we should force them off for the

llvm/lib/CodeGen/HardwareLoops.cpp

@@ -503,7 +503,7 @@ Value *HardwareLoop::InitLoopCount() {
 Value* HardwareLoop::InsertIterationSetup(Value *LoopCountInit) {
   IRBuilder<> Builder(BeginBB->getTerminator());
   if (BeginBB->getParent()->getAttributes().hasFnAttr(Attribute::StrictFP))
-    Builder.setIsFPConstrained(true);
+    Builder.resetModeToStrictFP(true);
   Type *Ty = LoopCountInit->getType();
   bool UsePhi = UsePHICounter || Opts.ForcePhi;
   Intrinsic::ID ID = UseLoopGuard
@@ -537,7 +537,7 @@ void HardwareLoop::InsertLoopDec() {
   IRBuilder<> CondBuilder(ExitBranch);
   if (ExitBranch->getParent()->getParent()->getAttributes().hasFnAttr(
           Attribute::StrictFP))
-    CondBuilder.setIsFPConstrained(true);
+    CondBuilder.resetModeToStrictFP(true);
 
   Value *Ops[] = { LoopDecrement };
   Value *NewCond = CondBuilder.CreateIntrinsic(Intrinsic::loop_decrement,
@@ -560,7 +560,7 @@ Instruction* HardwareLoop::InsertLoopRegDec(Value *EltsRem) {
   IRBuilder<> CondBuilder(ExitBranch);
   if (ExitBranch->getParent()->getParent()->getAttributes().hasFnAttr(
           Attribute::StrictFP))
-    CondBuilder.setIsFPConstrained(true);
+    CondBuilder.resetModeToStrictFP(true);
 
   Value *Ops[] = { EltsRem, LoopDecrement };
   Value *Call = CondBuilder.CreateIntrinsic(Intrinsic::loop_decrement_reg,

llvm/lib/IR/AsmWriter.cpp

@@ -100,6 +100,10 @@ static cl::opt<bool> PrintProfData(
     "print-prof-data", cl::Hidden,
     cl::desc("Pretty print perf data (branch weights, etc) when dumping"));
 
+static cl::opt<bool> PrintFPMemoryEffects(
+    "print-fp-memory-effects", cl::Hidden,
+    cl::desc("Pretty print floating-point memory effects when dumping"));
+
 // Make virtual table appear in this compilation unit.
 AssemblyAnnotationWriter::~AssemblyAnnotationWriter() = default;
 
@@ -4359,6 +4363,25 @@ void AssemblyWriter::printGCRelocateComment(const GCRelocateInst &Relocate) {
   Out << ")";
 }
 
+static void printFPMemoryEffects(raw_ostream &Out, MemoryEffects ME) {
+  ModRefInfo MR = ME.getModRef(IRMemLocation::InaccessibleMem);
+  Out << " ; fpe=[";
+  switch (MR) {
+  case ModRefInfo::NoModRef:
+    break;
+  case ModRefInfo::Ref:
+    Out << "r";
+    break;
+  case ModRefInfo::Mod:
+    Out << "w";
+    break;
+  case ModRefInfo::ModRef:
+    Out << "rw";
+    break;
+  }
+  Out << "]";
+}
+
 /// printInfoComment - Print a little comment after the instruction indicating
 /// which slot it occupies.
 void AssemblyWriter::printInfoComment(const Value &V) {
@@ -4389,30 +4412,14 @@ void AssemblyWriter::printInfoComment(const Value &V) {
   if (PrintInstAddrs)
     Out << " ; " << &V;
 
-  if (auto *CI = dyn_cast<CallInst>(&V))
-    if (Intrinsic::ID IID = CI->getIntrinsicID())
-      if (IntrinsicInst::isFloatingPointOperation(IID))
-        if (const BasicBlock *BB = CI->getParent())
-          if (const Function *F = BB->getParent())
-            if (F->hasFnAttribute(Attribute::StrictFP)) {
-              MemoryEffects ME = CI->getMemoryEffects();
-              ModRefInfo MR = ME.getModRef(IRMemLocation::InaccessibleMem);
-              Out << " ; fpe=[";
-              switch (MR) {
-              case ModRefInfo::NoModRef:
-                break;
-              case ModRefInfo::Ref:
-                Out << "r";
-                break;
-              case ModRefInfo::Mod:
-                Out << "w";
-                break;
-              case ModRefInfo::ModRef:
-                Out << "rw";
-                break;
-              }
-              Out << "]";
-            }
+  if (PrintFPMemoryEffects) {
+    if (auto *CI = dyn_cast<CallInst>(&V))
+      if (Intrinsic::ID IID = CI->getIntrinsicID())
+        if (const Function *F = CI->getFunction())
+          if (IntrinsicInst::isFloatingPointOperation(IID) &&
+              F->hasFnAttribute(Attribute::StrictFP))
+            printFPMemoryEffects(Out, CI->getMemoryEffects());
+  }
 }
 
 static void maybePrintCallAddrSpace(const Value *Operand, const Instruction *I,

llvm/lib/IR/FPEnv.cpp

@@ -76,27 +76,20 @@ std::optional<StringRef> convertRoundingModeToBundle(RoundingMode UseRounding) {
   std::optional<StringRef> RoundingStr;
   switch (UseRounding) {
   case RoundingMode::Dynamic:
-    RoundingStr = "dynamic";
-    break;
+    return "dynamic";
   case RoundingMode::NearestTiesToEven:
-    RoundingStr = "tonearest";
-    break;
+    return "tonearest";
   case RoundingMode::NearestTiesToAway:
-    RoundingStr = "tonearestaway";
-    break;
+    return "tonearestaway";
   case RoundingMode::TowardNegative:
-    RoundingStr = "downward";
-    break;
+    return "downward";
   case RoundingMode::TowardPositive:
-    RoundingStr = "upward";
-    break;
+    return "upward";
   case RoundingMode::TowardZero:
-    RoundingStr = "towardzero";
-    break;
+    return "towardzero";
   default:
-    break;
+    return std::nullopt;
   }
-  return RoundingStr;
 }
 
 std::optional<fp::ExceptionBehavior>
@@ -139,16 +132,14 @@ convertExceptionBehaviorToBundle(fp::ExceptionBehavior UseExcept) {
   std::optional<StringRef> ExceptStr;
   switch (UseExcept) {
   case fp::ebStrict:
-    ExceptStr = "strict";
-    break;
+    return "strict";
   case fp::ebIgnore:
-    ExceptStr = "ignore";
-    break;
+    return "ignore";
   case fp::ebMayTrap:
-    ExceptStr = "maytrap";
-    break;
+    return "maytrap";
+  default:
+    return std::nullopt;
   }
-  return ExceptStr;
 }
 
 Intrinsic::ID getConstrainedIntrinsicID(const Instruction &Instr) {

llvm/lib/IR/IRBuilder.cpp

@@ -125,7 +125,7 @@ CallInst *IRBuilderBase::CreateCall(FunctionType *FTy, Value *Callee,
   if (const auto *Func = dyn_cast<Function>(Callee))
     if (Intrinsic::ID ID = Func->getIntrinsicID())
       if (IntrinsicInst::isFloatingPointOperation(ID)) {
-        // If the builder has non-default floating-point options, add
+        // If the builder specifies non-default floating-point options, add
         // corresponding operand bundle unless a bundle with such tag is already
         // present.
         bool NeedRounding;
@@ -140,23 +140,24 @@ CallInst *IRBuilderBase::CreateCall(FunctionType *FTy, Value *Callee,
           assert(DefaultConstrainedExcept == fp::ebIgnore &&
                  "FP exception in default mode must be ignored");
         }
+        // Options specified by bundles have higher precedence.
+        for (const auto &Bundle : OpBundles) {
+          if (NeedRounding && Bundle.getTag() == "fp.round")
+            NeedRounding = false;
+          if (NeedExceptions && Bundle.getTag() == "fp.except")
+            NeedExceptions = false;
+        }
         if (NeedRounding || NeedExceptions) {
-          for (const auto &Bundle : OpBundles) {
-            if (NeedRounding && Bundle.getTag() == "fp.round")
-              NeedRounding = false;
-            if (NeedExceptions && Bundle.getTag() == "fp.except")
-              NeedExceptions = false;
-          }
-          if (NeedRounding || NeedExceptions) {
-            ActualBundles.append(OpBundles.begin(), OpBundles.end());
-            if (NeedRounding)
-              createRoundingBundle(ActualBundles, DefaultConstrainedRounding);
-            if (NeedExceptions)
-              createExceptionBundle(ActualBundles, DefaultConstrainedExcept);
-            ActualBundlesRef = ActualBundles;
-          }
+          ActualBundles.append(OpBundles.begin(), OpBundles.end());
+          if (NeedRounding)
+            createRoundingBundle(ActualBundles, DefaultConstrainedRounding);
+          if (NeedExceptions)
+            createExceptionBundle(ActualBundles, DefaultConstrainedExcept);
+          ActualBundlesRef = ActualBundles;
         }
         if (IsFPConstrained) {
+          // Due to potential reading FP exception bits, in strictfp mode the
+          // memory effects must include read/write access to FPE.
           MemoryEffects FME = Func->getMemoryEffects();
           NeedUpdateMemoryEffects = !FME.doesAccessInaccessibleMem();
         }

llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp

@@ -649,7 +649,7 @@ void AMDGPUAtomicOptimizerImpl::optimizeAtomic(Instruction &I,
   IRBuilder<> B(&I);
 
   if (AtomicRMWInst::isFPOperation(Op)) {
-    B.setIsFPConstrained(I.getFunction()->hasFnAttribute(Attribute::StrictFP));
+    B.resetModeToStrictFP(I.getFunction()->hasFnAttribute(Attribute::StrictFP));
   }
 
   // If we are in a pixel shader, because of how we have to mask out helper