Page MenuHomePhabricator

[Attributor] Look through allocated heap memory
AcceptedPublic

Authored by jdoerfert on Sep 2 2021, 9:55 AM.

Details

Summary

AAPointerInfo, and thereby other places, can look already through
internal global and stack memory. This patch enables them to look
through heap memory returned by functions with a noalias return.

In the future we can look through noalias arguments as well but that
will require AAIsDead to learn that such memory can be inspected by the
caller later on. We also need teach AAPointerInfo about dominance to
actually deal with memory that might not be null or undef
initialized. D106397 is a first step in that direction already.

Diff Detail

Event Timeline

jdoerfert created this revision.Sep 2 2021, 9:55 AM
jdoerfert requested review of this revision.Sep 2 2021, 9:55 AM
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
jdoerfert updated this revision to Diff 371203.Sep 7 2021, 3:18 PM

Add tests

jdoerfert retitled this revision from [Attributor][WIP] Look through allocated heap memory to [Attributor] Look through allocated heap memory.Sep 7 2021, 3:19 PM
jdoerfert edited the summary of this revision. (Show Details)
jdoerfert added a reviewer: kuter.
kuter added a comment.Sep 7 2021, 8:47 PM

Note: Some tests seem broken (heap_to_stack.ll) and clang format is needed.

Also, do you think we can optimize nested stuff ? for example if a pointer was stored to memory.
I think checkForAllUses can look through values that escape to memory, right ?

llvm/lib/Transforms/IPO/Attributor.cpp
210

I don't think this check is needed. isNoAliasFn seems to always return true if this is a allocation function.

Source code for isNoAliasFn

/// Tests if a value is a call or invoke to a function that returns a
/// NoAlias pointer (including malloc/calloc/realloc/strdup-like functions).
bool llvm::isNoAliasFn(const Value *V, const TargetLibraryInfo *TLI,
                       bool LookThroughBitCast) {
  // it's safe to consider realloc as noalias since accessing the original
  // pointer is undefined behavior
  return isAllocationFn(V, TLI, LookThroughBitCast) ||
         hasNoAliasAttr(V, LookThroughBitCast);
}
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
2459

Do you think we should use the AAReachability here ?
If the function can reach itself, then it is recursive.

Also, do you think we can optimize nested stuff ? for example if a pointer was stored to memory.
I think checkForAllUses can look through values that escape to memory, right ?

It "works" but I discovered that we need one more piece that I will put in here or separately.
Basically, we need to connect the use that is stored with the copies that are loaded.

llvm/lib/Transforms/IPO/Attributor.cpp
210

I don't understand why this would not be needed.

llvm/lib/Transforms/IPO/AttributorAttributes.cpp
2459

Yes, I'm looking into that already.

jdoerfert edited the summary of this revision. (Show Details)

Update test

jdoerfert updated this revision to Diff 372131.Sep 12 2021, 1:30 PM

Add a (currently failing) nested memory test

kuter accepted this revision.Sep 12 2021, 2:16 PM

LGTM

This revision is now accepted and ready to land.Sep 12 2021, 2:16 PM