Skip to content

Conversation

@CarolineConcatto
Copy link
Contributor

Extend MemorySSA’s clobber query to better distinguish calls that access inaccessible memory improving code motion opportunities in loops.

If both calls dont clobber Inaccessible Memory Location it can return from instructionClobbersQuery without setting ModeRefInfo. Otherwise it relies in the default behaviour to set ModRefInfo to Read and Write.

This enables LICM to hoist calls that modify inaccessible memory, improving code motion opportunities in loops.

@llvmbot llvmbot added llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Nov 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 24, 2025

@llvm/pr-subscribers-llvm-globalisel
@llvm/pr-subscribers-backend-amdgpu
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-analysis

Author: None (CarolineConcatto)

Changes

Extend MemorySSA’s clobber query to better distinguish calls that access inaccessible memory improving code motion opportunities in loops.

If both calls dont clobber Inaccessible Memory Location it can return from instructionClobbersQuery without setting ModeRefInfo. Otherwise it relies in the default behaviour to set ModRefInfo to Read and Write.

This enables LICM to hoist calls that modify inaccessible memory, improving code motion opportunities in loops.


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

3 Files Affected:

  • (modified) llvm/lib/Analysis/MemorySSA.cpp (+14)
  • (modified) llvm/lib/IR/Instructions.cpp (+1)
  • (added) llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll (+90)
diff --git a/llvm/lib/Analysis/MemorySSA.cpp b/llvm/lib/Analysis/MemorySSA.cpp
index 0b2e3fcfd76df..0f2747c8d71d7 100644
--- a/llvm/lib/Analysis/MemorySSA.cpp
+++ b/llvm/lib/Analysis/MemorySSA.cpp
@@ -277,6 +277,17 @@ static bool areLoadsReorderable(const LoadInst *Use,
   return !(SeqCstUse || MayClobberIsAcquire);
 }
 
+bool writeToInaccessibleMem(const CallBase *CallFirst,
+                             const CallBase *CallSecond) {
+
+  MemoryEffects ME1 = CallFirst->getMemoryEffects();
+  MemoryEffects ME2 = CallSecond->getMemoryEffects();
+  if (CallFirst->onlyAccessesInaccessibleMemory() ||
+      CallSecond->onlyAccessesInaccessibleMemory())
+    return !(ME1 & ME2 & MemoryEffects::writeOnly()).onlyReadsMemory();
+  return true;
+}
+
 template <typename AliasAnalysisType>
 static bool
 instructionClobbersQuery(const MemoryDef *MD, const MemoryLocation &UseLoc,
@@ -311,6 +322,9 @@ instructionClobbersQuery(const MemoryDef *MD, const MemoryLocation &UseLoc,
   }
 
   if (auto *CB = dyn_cast_or_null<CallBase>(UseInst)) {
+    if (auto *CU = dyn_cast_or_null<CallBase>(DefInst))
+      if (!writeToInaccessibleMem(CB, CU))
+        return false;
     ModRefInfo I = AA.getModRefInfo(DefInst, CB);
     return isModOrRefSet(I);
   }
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index cd39970f5111f..0a6ac96817517 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -693,6 +693,7 @@ void CallBase::setOnlyAccessesArgMemory() {
 bool CallBase::onlyAccessesInaccessibleMemory() const {
   return getMemoryEffects().onlyAccessesInaccessibleMem();
 }
+
 void CallBase::setOnlyAccessesInaccessibleMemory() {
   setMemoryEffects(getMemoryEffects() & MemoryEffects::inaccessibleMemOnly());
 }
diff --git a/llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll b/llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll
new file mode 100644
index 0000000000000..8b2ac7d8fdaa6
--- /dev/null
+++ b/llvm/test/Transforms/LICM/hoist-inaccesiblemem-call.ll
@@ -0,0 +1,90 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -aa-pipeline=basic-aa -passes='require<aa>,require<target-ir>,loop-mssa(licm)' < %s -S | FileCheck %s
+
+define void @inaccessible_hoist(ptr noalias %loc, ptr noalias %loc2){
+; CHECK-LABEL: define void @inaccessible_hoist(
+; CHECK-SAME: ptr noalias [[LOC:%.*]], ptr noalias [[LOC2:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[VAL:%.*]] = load i32, ptr [[LOC2]], align 4
+; CHECK-NEXT:    store i32 [[VAL]], ptr [[LOC]], align 4
+; CHECK-NEXT:    call void @fn_write_inaccessible_mem()
+; CHECK-NEXT:    call void @fn_read_inaccessible_mem()
+; CHECK-NEXT:    br label %[[FOR_BODY:.*]]
+; CHECK:       [[FOR_COND_CLEANUP:.*:]]
+; CHECK-NEXT:    ret void
+; CHECK:       [[FOR_BODY]]:
+; CHECK-NEXT:    br label %[[FOR_BODY]]
+;
+entry:
+  br label %for.body
+for.cond.cleanup:                                 ; preds = %for.body
+  ret void
+for.body:
+  %val = load i32, ptr %loc2
+  store i32 %val, ptr %loc
+  call void @fn_write_inaccessible_mem()
+  call void @fn_read_inaccessible_mem()
+  br label %for.body
+}
+
+
+define void @neg_inaccessible_hoist(ptr noalias %loc, ptr noalias %loc2){
+; CHECK-LABEL: define void @neg_inaccessible_hoist(
+; CHECK-SAME: ptr noalias [[LOC:%.*]], ptr noalias [[LOC2:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    [[VAL:%.*]] = load i32, ptr [[LOC2]], align 4
+; CHECK-NEXT:    store i32 [[VAL]], ptr [[LOC]], align 4
+; CHECK-NEXT:    br label %[[FOR_BODY:.*]]
+; CHECK:       [[FOR_BODY]]:
+; CHECK-NEXT:    call void @fn_write_inaccessible_mem()
+; CHECK-NEXT:    call void @fn_read_inaccessible_mem()
+; CHECK-NEXT:    call void @fn_readwrite_inaccessible_mem()
+; CHECK-NEXT:    br label %[[FOR_BODY]]
+;
+entry:
+  br label %for.body
+for.body:
+  %val = load i32, ptr %loc2
+  store i32 %val, ptr %loc
+  call void @fn_write_inaccessible_mem()
+  call void @fn_read_inaccessible_mem()
+  call void @fn_readwrite_inaccessible_mem()
+  br label %for.body
+}
+
+
+; Nothing should be hoisted from the loop because volatile
+; sets inaccessible memory to read write
+define void @neg_volatile(ptr %loc, ptr %loc2) {
+; CHECK-LABEL: define void @neg_volatile(
+; CHECK-SAME: ptr [[LOC:%.*]], ptr [[LOC2:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*:]]
+; CHECK-NEXT:    br label %[[LOOP:.*]]
+; CHECK:       [[LOOP]]:
+; CHECK-NEXT:    store volatile i32 0, ptr [[LOC]], align 4
+; CHECK-NEXT:    call void @fn_write_inaccessible_mem()
+; CHECK-NEXT:    call void @fn_read_inaccessible_mem()
+; CHECK-NEXT:    br label %[[LOOP]]
+;
+entry:
+  br label %loop
+
+loop:
+  %val = load i32, ptr %loc2
+  store volatile i32 0, ptr %loc
+  call void @fn_write_inaccessible_mem()
+  call void @fn_read_inaccessible_mem()
+  br label %loop
+}
+
+declare void @fn_write_inaccessible_mem()#0
+  memory(inaccessiblemem:  write)
+
+declare void @fn_read_inaccessible_mem()#0
+  memory(inaccessiblemem: read)
+
+declare void @fn_readwrite_inaccessible_mem()#0
+  memory(inaccessiblemem: readwrite)
+
+; Needs to set nounwind because of doesNotThrow
+attributes #0 = { mustprogress nofree norecurse nosync nounwind}

@github-actions
Copy link

github-actions bot commented Nov 24, 2025

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

@github-actions
Copy link

github-actions bot commented Nov 24, 2025

🐧 Linux x64 Test Results

  • 186662 tests passed
  • 4884 tests skipped

Extend `MemorySSA`’s clobber query to better distinguish calls that
access inaccessible memory improving code motion opportunities in loops.

If both calls dont clobber Inaccessible Memory Location it can return
from instructionClobbersQuery without setting ModeRefInfo.
Otherwise it relies in the default behaviour to set ModRefInfo to
Read and Write.

This enables LICM to hoist calls that modify inaccessible memory,
improving code motion opportunities in loops.
Comment on lines 285 to 286
if (CallFirst->onlyAccessesInaccessibleMemory() ||
CallSecond->onlyAccessesInaccessibleMemory())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't onlyAccessesInaccessibleMemory implemented in terms of MemoryEffects? Can you keep all of this in terms of MemoryEffects?

Copy link
Contributor Author

@CarolineConcatto CarolineConcatto Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @arsenm,
Not sure I understand the question.
onlyAccessesInaccessibleMemory is checking the memory effect of the call. It calls onlyAccessesInaccessibleMem that check if the memory effect only uses Inaccessible memory:
return getWithoutLoc(Location::InaccessibleMem).doesNotAccessMemory();
Do you mean I should look at a Data?
But AFAIU that is what this does indirectly. It checks if any read or/and write is done in the Inaccessible memory locations

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean stop using this wrapper, and directly query the MemoryEffects

Copy link
Contributor Author

@CarolineConcatto CarolineConcatto Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why exposing memory effects for both calls inside instructionClobbersQuery function is better than hidden inside the function hasInaccessibleMemoryClobber.
Do you mind explaining what is the reason?
I could check the memory effects in instructionClobbersQuery, I would just want to understand why you are asking for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @arsenm ,
I did remove the function hasInaccessibleMemoryClobber.
Does this solves this comment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:AMDGPU llvm:analysis Includes value tracking, cost tables and constant folding llvm:globalisel llvm:ir llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants