Skip to content

[LSV] Merge contiguous chains across scalar types #154069

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[LSV] Merge contiguous chains across scalar types #154069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions llvm/include/llvm/Transforms/Utils/Local.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,10 @@ LLVM_ABI void combineAAMetadata(Instruction *K, const Instruction *J);
/// replacement for the source instruction).
LLVM_ABI void copyMetadataForLoad(LoadInst &Dest, const LoadInst &Source);

/// Copy the metadata from the source instruction to the destination (the
/// replacement for the source instruction).
LLVM_ABI void copyMetadataForStore(StoreInst &Dest, const StoreInst &Source);

/// Patch the replacement so that it is not more restrictive than the value
/// being replaced. It assumes that the replacement does not get moved from
/// its original position.
Expand Down
42 changes: 42 additions & 0 deletions llvm/lib/Transforms/Utils/Local.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3153,6 +3153,48 @@ void llvm::copyMetadataForLoad(LoadInst &Dest, const LoadInst &Source) {
}
}

void llvm::copyMetadataForStore(StoreInst &Dest, const StoreInst &Source) {
SmallVector<std::pair<unsigned, MDNode *>, 8> MD;
Source.getAllMetadata(MD);
MDBuilder MDB(Dest.getContext());
Type *NewType = Dest.getType();
for (const auto &MDPair : MD) {
unsigned ID = MDPair.first;
MDNode *N = MDPair.second;
switch (ID) {
case LLVMContext::MD_dbg:
case LLVMContext::MD_prof:
case LLVMContext::MD_tbaa_struct:
case LLVMContext::MD_alias_scope:
case LLVMContext::MD_noalias:
case LLVMContext::MD_nontemporal:
case LLVMContext::MD_access_group:
case LLVMContext::MD_noundef:
case LLVMContext::MD_noalias_addrspace:
case LLVMContext::MD_mem_parallel_loop_access:
Dest.setMetadata(ID, N);
break;

case LLVMContext::MD_tbaa: {
MDNode *NewTyNode =
MDB.createTBAAScalarTypeNode(NewType->getStructName(), N);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what you're trying to do here, but it's definitely not correct.

Also NewType itself is incorrect, as it's the store type, which is always void, rather than the store value type.

Dest.setMetadata(LLVMContext::MD_tbaa, NewTyNode);
break;
}
case LLVMContext::MD_nonnull:
break;

case LLVMContext::MD_align:
case LLVMContext::MD_dereferenceable:
case LLVMContext::MD_dereferenceable_or_null:
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of the metadata listed here is not applicable to stores at all (at least noundef, nonnull, align, dereferenceable and deferenceable_or_null).

Copy link
Contributor

Choose a reason for hiding this comment

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

You could probably generalize copyMetadataForLoad to something like copyMetadataForAccess that handles both, as the handling is going to be essentially the same, but just copying the code for stores doesn't make sense.

// These only directly apply if the new type is also a pointer.
if (NewType->isPointerTy())
Dest.setMetadata(ID, N);
break;
}
}
}

void llvm::patchReplacementInstruction(Instruction *I, Value *Repl) {
auto *ReplInst = dyn_cast<Instruction>(Repl);
if (!ReplInst)
Expand Down
Loading