Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Target][AMDGPU] Fix TSan error on AMDGPU Target. #79529

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

MaheshRavishankar
Copy link
Contributor

Updating the value of the global flag within the code was flagged as a TSAN error. Fixing that.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 26, 2024

@llvm/pr-subscribers-backend-amdgpu

Author: None (MaheshRavishankar)

Changes

Updating the value of the global flag within the code was flagged as a TSAN error. Fixing that.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp (+18-10)
  • (modified) llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.h (+3-1)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp b/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
index 0c759e7f3b0957e..317a5bb7462a8bb 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.cpp
@@ -46,12 +46,12 @@ char &llvm::AMDGPUResourceUsageAnalysisID = AMDGPUResourceUsageAnalysis::ID;
 // In code object v4 and older, we need to tell the runtime some amount ahead of
 // time if we don't know the true stack size. Assume a smaller number if this is
 // only due to dynamic / non-entry block allocas.
-static cl::opt<uint32_t> AssumedStackSizeForExternalCall(
+static cl::opt<uint32_t> clAssumedStackSizeForExternalCall(
     "amdgpu-assume-external-call-stack-size",
     cl::desc("Assumed stack use of any external call (in bytes)"), cl::Hidden,
     cl::init(16384));
 
-static cl::opt<uint32_t> AssumedStackSizeForDynamicSizeObjects(
+static cl::opt<uint32_t> clAssumedStackSizeForDynamicSizeObjects(
     "amdgpu-assume-dynamic-stack-object-size",
     cl::desc("Assumed extra stack use if there are any "
              "variable sized objects (in bytes)"),
@@ -112,11 +112,15 @@ bool AMDGPUResourceUsageAnalysis::runOnModule(Module &M) {
 
   // By default, for code object v5 and later, track only the minimum scratch
   // size
+  uint32_t AssumedStackSizeForDynamicSizeObjects =
+      clAssumedStackSizeForDynamicSizeObjects.getDefault().getValue();
+  uint32_t AssumedStackSizeForExternalCall =
+      clAssumedStackSizeForExternalCall.getDefault().getValue();
   if (AMDGPU::getAMDHSACodeObjectVersion(M) >= AMDGPU::AMDHSA_COV5 ||
       STI.getTargetTriple().getOS() == Triple::AMDPAL) {
-    if (!AssumedStackSizeForDynamicSizeObjects.getNumOccurrences())
+    if (!clAssumedStackSizeForDynamicSizeObjects.getNumOccurrences())
       AssumedStackSizeForDynamicSizeObjects = 0;
-    if (!AssumedStackSizeForExternalCall.getNumOccurrences())
+    if (!clAssumedStackSizeForExternalCall.getNumOccurrences())
       AssumedStackSizeForExternalCall = 0;
   }
 
@@ -132,7 +136,8 @@ bool AMDGPUResourceUsageAnalysis::runOnModule(Module &M) {
         CallGraphResourceInfo.insert(std::pair(F, SIFunctionResourceInfo()));
     SIFunctionResourceInfo &Info = CI.first->second;
     assert(CI.second && "should only be called once per function");
-    Info = analyzeResourceUsage(*MF, TM);
+    Info = analyzeResourceUsage(*MF, TM, AssumedStackSizeForDynamicSizeObjects,
+                                AssumedStackSizeForExternalCall);
     HasIndirectCall |= Info.HasIndirectCall;
   }
 
@@ -152,7 +157,8 @@ bool AMDGPUResourceUsageAnalysis::runOnModule(Module &M) {
     SIFunctionResourceInfo &Info = CI.first->second;
     MachineFunction *MF = MMI.getMachineFunction(*F);
     assert(MF && "function must have been generated already");
-    Info = analyzeResourceUsage(*MF, TM);
+    Info = analyzeResourceUsage(*MF, TM, AssumedStackSizeForDynamicSizeObjects,
+                                AssumedStackSizeForExternalCall);
     HasIndirectCall |= Info.HasIndirectCall;
   }
 
@@ -164,7 +170,9 @@ bool AMDGPUResourceUsageAnalysis::runOnModule(Module &M) {
 
 AMDGPUResourceUsageAnalysis::SIFunctionResourceInfo
 AMDGPUResourceUsageAnalysis::analyzeResourceUsage(
-    const MachineFunction &MF, const TargetMachine &TM) const {
+    const MachineFunction &MF, const TargetMachine &TM,
+    uint32_t AssumedStackSizeForDynamicSizeObjects,
+    uint32_t AssumedStackSizeForExternalCall) const {
   SIFunctionResourceInfo Info;
 
   const SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
@@ -541,9 +549,9 @@ AMDGPUResourceUsageAnalysis::analyzeResourceUsage(
             // directly call the tail called function. If a kernel directly
             // calls a tail recursive function, we'll assume maximum stack size
             // based on the regular call instruction.
-            CalleeFrameSize =
-              std::max(CalleeFrameSize,
-                       static_cast<uint64_t>(AssumedStackSizeForExternalCall));
+            CalleeFrameSize = std::max(
+                CalleeFrameSize,
+                static_cast<uint64_t>(AssumedStackSizeForExternalCall));
           }
         }
 
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.h b/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.h
index df0789e471c16a5..93b5c3ce71e517d 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUResourceUsageAnalysis.h
@@ -73,7 +73,9 @@ struct AMDGPUResourceUsageAnalysis : public ModulePass {
 
 private:
   SIFunctionResourceInfo analyzeResourceUsage(const MachineFunction &MF,
-                                              const TargetMachine &TM) const;
+                                              const TargetMachine &TM,
+    uint32_t AssumedStackSizeForDynamicSizeObjects,
+    uint32_t AssumedStackSizeForExternalCall) const;
   void propagateIndirectCallRegisterUsage();
 
   DenseMap<const Function *, SIFunctionResourceInfo> CallGraphResourceInfo;

Copy link

github-actions bot commented Jan 26, 2024

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

Updating the value of the global flag within the code was flagged as a
TSAN error. Fixing that.
@MaheshRavishankar MaheshRavishankar merged commit df5e431 into llvm:main Jan 26, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants