[llvm] [IR][NFC] Add LoadStoreInstAttributes to copy load/store attrs (PR #206470)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-ir

Author: Harrison Hao (harrisonGPU)

<details>
<summary>Changes</summary>

Introduce a small `LoadStoreInstAttributes` struct plus get/setAttributes on
`LoadInst` and `StoreInst` so volatile/align/ordering/syncscope can be copied
together instead of one field at a time. Switch the obvious load->load and
store->store clone sites over to it.

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


4 Files Affected:

- (modified) llvm/include/llvm/IR/Instructions.h (+34) 
- (modified) llvm/lib/CodeGen/AtomicExpandPass.cpp (+2-6) 
- (modified) llvm/lib/Target/AMDGPU/AMDGPUSwLowerLDS.cpp (+2-2) 
- (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (+4-6) 


``````````diff
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index f43a9b5c1acc2..1d64c19aae952 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -172,6 +172,14 @@ class AllocaInst : public UnaryInstruction {
   }
 };
 
+/// A structure representing the attributes of a load or store instruction.
+struct LoadStoreInstAttributes {
+  bool IsVolatile = false;
+  Align Alignment;
+  AtomicOrdering Ordering = AtomicOrdering::NotAtomic;
+  SyncScope::ID SSID = SyncScope::System;
+};
+
 //===----------------------------------------------------------------------===//
 //                                LoadInst Class
 //===----------------------------------------------------------------------===//
@@ -249,6 +257,19 @@ class LoadInst : public UnaryInstruction {
     setSyncScopeID(SSID);
   }
 
+  /// Returns the attributes of this load instruction.
+  LoadStoreInstAttributes getAttributes() const {
+    return {isVolatile(), getAlign(), getOrdering(), getSyncScopeID()};
+  }
+
+  /// Sets the attributes of this load instruction.
+  void setAttributes(const LoadStoreInstAttributes &Attrs) {
+    setVolatile(Attrs.IsVolatile);
+    setAlignment(Attrs.Alignment);
+    setOrdering(Attrs.Ordering);
+    setSyncScopeID(Attrs.SSID);
+  }
+
   bool isSimple() const { return !isAtomic() && !isVolatile(); }
 
   bool isUnordered() const {
@@ -373,6 +394,19 @@ class StoreInst : public Instruction {
     setSyncScopeID(SSID);
   }
 
+  /// Returns the attributes of this store instruction.
+  LoadStoreInstAttributes getAttributes() const {
+    return {isVolatile(), getAlign(), getOrdering(), getSyncScopeID()};
+  }
+
+  /// Sets the attributes of this store instruction.
+  void setAttributes(const LoadStoreInstAttributes &Attrs) {
+    setVolatile(Attrs.IsVolatile);
+    setAlignment(Attrs.Alignment);
+    setOrdering(Attrs.Ordering);
+    setSyncScopeID(Attrs.SSID);
+  }
+
   bool isSimple() const { return !isAtomic() && !isVolatile(); }
 
   bool isUnordered() const {
diff --git a/llvm/lib/CodeGen/AtomicExpandPass.cpp b/llvm/lib/CodeGen/AtomicExpandPass.cpp
index 059c0d5cb7b8b..0767d0baad287 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -564,9 +564,7 @@ LoadInst *AtomicExpandImpl::convertAtomicLoadToIntegerType(LoadInst *LI) {
   Value *Addr = LI->getPointerOperand();
 
   auto *NewLI = Builder.CreateLoad(NewTy, Addr);
-  NewLI->setAlignment(LI->getAlign());
-  NewLI->setVolatile(LI->isVolatile());
-  NewLI->setAtomic(LI->getOrdering(), LI->getSyncScopeID());
+  NewLI->setAttributes(LI->getAttributes());
   LLVM_DEBUG(dbgs() << "Replaced " << *LI << " with " << *NewLI << "\n");
 
   Value *NewVal = LI->getType()->isPtrOrPtrVectorTy()
@@ -720,9 +718,7 @@ StoreInst *AtomicExpandImpl::convertAtomicStoreToIntegerType(StoreInst *SI) {
   Value *Addr = SI->getPointerOperand();
 
   StoreInst *NewSI = Builder.CreateStore(NewVal, Addr);
-  NewSI->setAlignment(SI->getAlign());
-  NewSI->setVolatile(SI->isVolatile());
-  NewSI->setAtomic(SI->getOrdering(), SI->getSyncScopeID());
+  NewSI->setAttributes(SI->getAttributes());
   LLVM_DEBUG(dbgs() << "Replaced " << *SI << " with " << *NewSI << "\n");
   SI->eraseFromParent();
   return NewSI;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUSwLowerLDS.cpp b/llvm/lib/Target/AMDGPU/AMDGPUSwLowerLDS.cpp
index f410fa97f90ea..b2e3004bd5cad 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUSwLowerLDS.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUSwLowerLDS.cpp
@@ -696,7 +696,7 @@ void AMDGPUSwLowerLDS::translateLDSMemoryOperationsToGlobalMemory(
           getTranslatedGlobalMemoryPtrOfLDS(LoadMallocPtr, LIOperand);
       LoadInst *NewLI = IRB.CreateAlignedLoad(LI->getType(), Replacement,
                                               LI->getAlign(), LI->isVolatile());
-      NewLI->setAtomic(LI->getOrdering(), LI->getSyncScopeID());
+      NewLI->setAttributes(LI->getAttributes());
       AsanInfo.Instructions.insert(NewLI);
       LI->replaceAllUsesWith(NewLI);
       LI->eraseFromParent();
@@ -706,7 +706,7 @@ void AMDGPUSwLowerLDS::translateLDSMemoryOperationsToGlobalMemory(
           getTranslatedGlobalMemoryPtrOfLDS(LoadMallocPtr, SIOperand);
       StoreInst *NewSI = IRB.CreateAlignedStore(
           SI->getValueOperand(), Replacement, SI->getAlign(), SI->isVolatile());
-      NewSI->setAtomic(SI->getOrdering(), SI->getSyncScopeID());
+      NewSI->setAttributes(SI->getAttributes());
       AsanInfo.Instructions.insert(NewSI);
       SI->replaceAllUsesWith(NewSI);
       SI->eraseFromParent();
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index e6481c89c0265..2055ffb3950b7 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -605,7 +605,7 @@ LoadInst *InstCombinerImpl::combineLoadToNewType(LoadInst &LI, Type *NewTy,
   LoadInst *NewLoad =
       Builder.CreateAlignedLoad(NewTy, LI.getPointerOperand(), LI.getAlign(),
                                 LI.isVolatile(), LI.getName() + Suffix);
-  NewLoad->setAtomic(LI.getOrdering(), LI.getSyncScopeID());
+  NewLoad->setAttributes(LI.getAttributes());
   copyMetadataForLoad(*NewLoad, LI);
   return NewLoad;
 }
@@ -624,7 +624,7 @@ static StoreInst *combineStoreToNewValue(InstCombinerImpl &IC, StoreInst &SI,
 
   StoreInst *NewStore =
       IC.Builder.CreateAlignedStore(V, Ptr, SI.getAlign(), SI.isVolatile());
-  NewStore->setAtomic(SI.getOrdering(), SI.getSyncScopeID());
+  NewStore->setAttributes(SI.getAttributes());
   for (const auto &MDPair : MD) {
     unsigned ID = MDPair.first;
     MDNode *N = MDPair.second;
@@ -1166,10 +1166,8 @@ Instruction *InstCombinerImpl::visitLoadInst(LoadInst &LI) {
         LoadInst *V2 = Builder.CreateLoad(LI.getType(), LoadOp2,
                                           LoadOp2->getName() + ".val");
         assert(LI.isUnordered() && "implied by above");
-        V1->setAlignment(Alignment);
-        V1->setAtomic(LI.getOrdering(), LI.getSyncScopeID());
-        V2->setAlignment(Alignment);
-        V2->setAtomic(LI.getOrdering(), LI.getSyncScopeID());
+        V1->setAttributes(LI.getAttributes());
+        V2->setAttributes(LI.getAttributes());
         // It is safe to copy any metadata that does not trigger UB. Copy any
         // poison-generating metadata.
         V1->copyMetadata(LI, Metadata::PoisonGeneratingIDs);

``````````

</details>


https://github.com/llvm/llvm-project/pull/206470