AMDGPU: Stop setting attributes based on TargetOptions

[arsenm]

Summary

Having arbitrary passes looking at the TargetOptions is pretty
messy. This was also disregarding if a function already had an
explicit attribute setting on it. opt/llc now add the attributes to
functions that don't specify the attribute. clang and lld do not call
the function to do this, which they maybe should.

This was also treating unsafe-fp-math as implying the others, and
setting the other attributes based on it. This is not done anywhere
else, and I'm not sure is correct based on the current description of
the option bit.

Effectively reverts 1d8cf2be89087a2babc1dc38b16040fad0a555e2

[rampitec]

clang and lld do not call the function to do this, which they maybe should.

Should we first fix clang then?

[arsenm]

In D71259#1777574, @rampitec wrote:

clang and lld do not call the function to do this, which they maybe should.

Should we first fix clang then?

They probably shouldn't be using these flags, but I don't know where to diagnose it either. They can properly set the function attributes themselves. The purpose of these flags is to help writing lit tests

[rampitec]

In D71259#1777599, @arsenm wrote:

In D71259#1777574, @rampitec wrote:

clang and lld do not call the function to do this, which they maybe should.

Should we first fix clang then?

They probably shouldn't be using these flags, but I don't know where to diagnose it either. They can properly set the function attributes themselves. The purpose of these flags is to help writing lit tests

We should somehow translate opencl options into user code. I agree these should not be set on library functions. If clang properly set attributes now then this code indeed needs to be removed.

[arsenm]

In D71259#1777687, @rampitec wrote:

In D71259#1777599, @arsenm wrote:

In D71259#1777574, @rampitec wrote:

clang and lld do not call the function to do this, which they maybe should.

Should we first fix clang then?

They probably shouldn't be using these flags, but I don't know where to diagnose it either. They can properly set the function attributes themselves. The purpose of these flags is to help writing lit tests

We should somehow translate opencl options into user code. I agree these should not be set on library functions. If clang properly set attributes now then this code indeed needs to be removed.

The user facing flags should set the function attributes. clang can propagate the attributes into the OpenCL library when linked, but we currently aren't using clang to do the linking

[rampitec]

In D71259#1777733, @arsenm wrote:

In D71259#1777687, @rampitec wrote:

In D71259#1777599, @arsenm wrote:

In D71259#1777574, @rampitec wrote:

clang and lld do not call the function to do this, which they maybe should.

Should we first fix clang then?

They probably shouldn't be using these flags, but I don't know where to diagnose it either. They can properly set the function attributes themselves. The purpose of these flags is to help writing lit tests

We should somehow translate opencl options into user code. I agree these should not be set on library functions. If clang properly set attributes now then this code indeed needs to be removed.

The user facing flags should set the function attributes. clang can propagate the attributes into the OpenCL library when linked, but we currently aren't using clang to do the linking

I do not think it really should propagate attributes into the library. LGTM.

[arsenm]

348735b723048411167b2244ebf5e2cd19bd869b

llvm/lib/Target/AMDGPU/AMDGPU.h

 FunctionPass *createSIWholeQuadModePass();
 FunctionPass *createSIFixControlFlowLiveIntervalsPass();
 FunctionPass *createSIOptimizeExecMaskingPreRAPass();
 FunctionPass *createSIFixSGPRCopiesPass();
 FunctionPass *createSIMemoryLegalizerPass();
 FunctionPass *createSIInsertWaitcntsPass();
 FunctionPass *createSIPreAllocateWWMRegsPass();
 FunctionPass *createSIFormMemoryClausesPass();
-FunctionPass *createAMDGPUSimplifyLibCallsPass(const TargetOptions &,
-                                               const TargetMachine *);
+FunctionPass *createAMDGPUSimplifyLibCallsPass(const TargetMachine *);
 FunctionPass *createAMDGPUUseNativeCallsPass();
 FunctionPass *createAMDGPUCodeGenPreparePass();
 FunctionPass *createAMDGPUMachineCFGStructurizerPass();
 FunctionPass *createAMDGPUPropagateAttributesEarlyPass(const TargetMachine *);
 ModulePass *createAMDGPUPropagateAttributesLatePass(const TargetMachine *);
 FunctionPass *createAMDGPURewriteOutArgumentsPass();
 FunctionPass *createSIModeRegisterPass();
 

llvm/lib/Target/AMDGPU/AMDGPULibCalls.cpp

 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/ValueSymbolTable.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
-#include "llvm/Target/TargetOptions.h"
 #include <cmath>
 #include <vector>
 
 #define DEBUG_TYPE "amdgpu-simplifylib"
 
 using namespace llvm;
 
 static cl::opt<bool> EnablePreLink("amdgpu-prelink",

 };
 
 } // end llvm namespace
 
 namespace {
 
   class AMDGPUSimplifyLibCalls : public FunctionPass {
 
-  const TargetOptions Options;
-
   AMDGPULibCalls Simplifier;
 
   public:
     static char ID; // Pass identification
 
-    AMDGPUSimplifyLibCalls(const TargetOptions &Opt = TargetOptions(),
-                           const TargetMachine *TM = nullptr)
-      : FunctionPass(ID), Options(Opt), Simplifier(TM) {
+    AMDGPUSimplifyLibCalls(const TargetMachine *TM = nullptr)
+      : FunctionPass(ID), Simplifier(TM) {
       initializeAMDGPUSimplifyLibCallsPass(*PassRegistry::getPassRegistry());
     }
 
     void getAnalysisUsage(AnalysisUsage &AU) const override {
       AU.addRequired<AAResultsWrapperPass>();
     }
 
     bool runOnFunction(Function &M) override;
if (hasTwoResults) {
     new StoreInst(nval1, aCI->getArgOperand(1), aCI);
   }
 
   replaceCall(nval0);
   return true;
 }
 
 // Public interface to the Simplify LibCalls pass.
-FunctionPass *llvm::createAMDGPUSimplifyLibCallsPass(const TargetOptions &Opt,
-                                                     const TargetMachine *TM) {
-  return new AMDGPUSimplifyLibCalls(Opt, TM);
+FunctionPass *llvm::createAMDGPUSimplifyLibCallsPass(const TargetMachine *TM) {
+  return new AMDGPUSimplifyLibCalls(TM);
 }
 
 FunctionPass *llvm::createAMDGPUUseNativeCallsPass() {
   return new AMDGPUUseNativeCalls();
 }
 
-static bool setFastFlags(Function &F, const TargetOptions &Options) {
-  AttrBuilder B;
-
-  if (Options.UnsafeFPMath || Options.NoInfsFPMath)
-    B.addAttribute("no-infs-fp-math", "true");
-  if (Options.UnsafeFPMath || Options.NoNaNsFPMath)
-    B.addAttribute("no-nans-fp-math", "true");
-  if (Options.UnsafeFPMath) {
-    B.addAttribute("less-precise-fpmad", "true");
-    B.addAttribute("unsafe-fp-math", "true");
-  }
-
-  if (!B.hasAttributes())
-    return false;
-
-  F.addAttributes(AttributeList::FunctionIndex, B);
-
-  return true;
-}
-
 bool AMDGPUSimplifyLibCalls::runOnFunction(Function &F) {
   if (skipFunction(F))
     return false;
 
   bool Changed = false;
   auto AA = &getAnalysis<AAResultsWrapperPass>().getAAResults();
 
   LLVM_DEBUG(dbgs() << "AMDIC: process function ";
              F.printAsOperand(dbgs(), false, F.getParent()); dbgs() << '\n';);
 
-  if (!EnablePreLink)
-    Changed |= setFastFlags(F, Options);
-
   for (auto &BB : F) {
     for (BasicBlock::iterator I = BB.begin(), E = BB.end(); I != E; ) {
       // Ignore non-calls.
       CallInst *CI = dyn_cast<CallInst>(I);
       ++I;
       if (!CI) continue;
 
       // Ignore indirect calls.

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp

[Internalize, EarlyInline, AMDGPUAA, this](const PassManagerBuilder &,
       if (Internalize) {
         PM.add(createInternalizePass(mustPreserveGV));
         PM.add(createGlobalDCEPass());
       }
       if (EarlyInline)
         PM.add(createAMDGPUAlwaysInlinePass(false));
   });
 
-  const auto &Opt = Options;
   Builder.addExtension(
     PassManagerBuilder::EP_EarlyAsPossible,
-    [AMDGPUAA, LibCallSimplify, &Opt, this](const PassManagerBuilder &,
-                                            legacy::PassManagerBase &PM) {
+    [AMDGPUAA, LibCallSimplify, this](const PassManagerBuilder &,
       if (AMDGPUAA) {
         PM.add(createAMDGPUAAWrapperPass());
         PM.add(createAMDGPUExternalAAWrapperPass());
       }
       PM.add(llvm::createAMDGPUPropagateAttributesEarlyPass(this));
       PM.add(llvm::createAMDGPUUseNativeCallsPass());
       if (LibCallSimplify)
-        PM.add(llvm::createAMDGPUSimplifyLibCallsPass(Opt, this));
+        PM.add(llvm::createAMDGPUSimplifyLibCallsPass(this));
   });
 
   Builder.addExtension(
     PassManagerBuilder::EP_CGSCCOptimizerLate,
     [](const PassManagerBuilder &, legacy::PassManagerBase &PM) {
       // Add infer address spaces pass to the opt pipeline after inlining
       // but before SROA to increase SROA opportunities.
       PM.add(createInferAddressSpacesPass());

llvm/test/CodeGen/AMDGPU/inline-attr.ll

 ; RUN: opt -mtriple=amdgcn--amdhsa -S -O3 -enable-unsafe-fp-math %s  | FileCheck -check-prefix=GCN -check-prefix=UNSAFE %s
 ; RUN: opt -mtriple=amdgcn--amdhsa -S -O3 -enable-no-nans-fp-math %s | FileCheck -check-prefix=GCN -check-prefix=NONANS %s
 ; RUN: opt -mtriple=amdgcn--amdhsa -S -O3 -enable-no-infs-fp-math %s | FileCheck -check-prefix=GCN -check-prefix=NOINFS %s
 
 ; GCN: define float @foo(float %x) local_unnamed_addr #0 {
 ; GCN: define amdgpu_kernel void @caller(float addrspace(1)* nocapture %p) local_unnamed_addr #1 {
 ; GCN: %mul.i = fmul float %load, 1.500000e+01
 
-; UNSAFE: attributes #0 = { norecurse nounwind readnone "less-precise-fpmad"="true" "no-infs-fp-math"="true" "no-nans-fp-math"="true" "unsafe-fp-math"="true" }
-; UNSAFE: attributes #1 = { nofree norecurse nounwind "less-precise-fpmad"="true" "no-infs-fp-math"="true" "no-nans-fp-math"="true" "unsafe-fp-math"="true" }
+; UNSAFE: attributes #0 = { norecurse nounwind readnone "unsafe-fp-math"="true" }
+; UNSAFE: attributes #1 = { nofree norecurse nounwind "less-precise-fpmad"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "unsafe-fp-math"="true" }
 
 ; NOINFS: attributes #0 = { norecurse nounwind readnone "no-infs-fp-math"="true" }
 ; NOINFS: attributes #1 = { nofree norecurse nounwind "less-precise-fpmad"="false" "no-infs-fp-math"="true" "no-nans-fp-math"="false" "unsafe-fp-math"="false" }
 
 ; NONANS: attributes #0 = { norecurse nounwind readnone "no-nans-fp-math"="true" }
 ; NONANS: attributes #1 = { nofree norecurse nounwind "less-precise-fpmad"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="true" "unsafe-fp-math"="false" }
 
 define float @foo(float %x) #0 {