Page MenuHomePhabricator

implantation example for new AliasAnalysis functions (getAddressesDistance + ModRefSameBuffer)
Needs ReviewPublic

Authored by atheel.ma on May 15 2018, 8:45 PM.

Details

Summary

just a "First" implantation , maybe need to get all common code between aliasCheck() and commonBuffCheck() to a common function

Diff Detail

Repository
rL LLVM

Event Timeline

atheel.ma created this revision.May 15 2018, 8:45 PM
atheel.ma added a comment.EditedMay 15 2018, 8:46 PM

The use is inside a target specific function:

  for (MachineMemOperand *MMOa : MIa.memoperands()) {
    for (MachineMemOperand *MMOb : MIb.memoperands()) {
      const Value *Va = MMOa->getValue();
      const Value *Vb = MMOb->getValue();
      const PseudoSourceValue *Pa = MMOa->getPseudoValue();
      const PseudoSourceValue *Pb = MMOb->getPseudoValue();

      if (Va && Vb) {
        // Ask alias analysis (if there's one available) what's going on if
        // we have access to IR values.
        assert(!Pa && !Pb && "IR vs pseudo value inconsistency");
        uint64_t Sizea = MMOa->getSize();
        uint64_t Sizeb = MMOb->getSize();

        // vpld has sometimes UnknownSize
        if ((!CII->isUsingAllBanks(MIa) && !CII->isUsingAllBanks(MIb)) &&
            (MMOa->getSize() == MemoryLocation::UnknownSize ||
             MMOb->getSize() == MemoryLocation::UnknownSize)) {
          continue;
        }

        MemoryLocation Loca = MemoryLocation(Va, Sizea, MMOa->getAAInfo());
        MemoryLocation Locb = MemoryLocation(Vb, Sizeb, MMOb->getAAInfo());
		int64_t ptr1Offset = MMOa->getPointerInfo().Offset;
		int64_t ptr2Offset = MMOb->getPointerInfo().Offset;
        if (AA->ModRefSameBuffer(Loca, ptr1Offset, Locb, ptr2Offset)) {

			Optional<int64_t> dis = AA->getAddressesDistance(Loca,
				ptr1Offset, Locb, ptr2Offset);

          if ((CII->isUsingAllBanks(MIa) || CII->isUsingAllBanks(MIb))) {
            return true;
          }
          // need to take care with dis<0 *dis%64 => ((*dis%64)+64)%64
          if (dis != None && ((*dis > 0 && Sizeb > (*dis % 64)) ||
                              (*dis < 0 && Sizea > ((*dis % 64) + 64) % 64))) {
            return true;
          }
        }
      }
    }
  }

Thank you for the implementation patch!

AFAICT this is a fairly big change, and I'd like it to get more visibility and feedback. Could you please send an RFC to llvm-dev list with a short description and links to the two patches?

I'd like to understand if there's the opportunity to improve existing AA interfaces (e.g. what's missing in aliasCheck) here versus adding new ones. There's a lot of duplicated code there.
Let's continue the discussion on llvm-dev, please.

thanks,
mail sent to llvm-dev,
ofcurse Im aware of the "duplicated code", this is only a temporary implementation,
Okm will discuss it on llvm-dev .

xbolva00 added a subscriber: xbolva00.EditedMay 16 2018, 11:42 PM

Maybe add more documentation, examples how to use these new API?

include/llvm/Analysis/BasicAliasAnalysis.h
91

Fix comment, you return bool value.

xbolva00 added inline comments.May 16 2018, 11:58 PM
include/llvm/Analysis/BasicAliasAnalysis.h
152

..common buffer queries...

153

Convert to more C++ish code via "using MyType = ..."?

lib/Analysis/BasicAliasAnalysis.cpp
625

isa<...> is enough here, no?

You can use still getParent function with "V"

757

if (!ptr)...

760

Same as above

832

Why extra if ((...))?

1745

Is this loop correct? Why start at 1? And why not "i < e"?

Anyway, range based iteration is nicer, you should chrck if possible here

for (Value *V : V1Srcs) { ... }

Fix some comments
remove duplicated functions

atheel.ma marked 3 inline comments as done.EditedMay 21 2018, 11:31 PM

O1 == nullptr is taken from aliasCheck()
If I want to change it to !nullptr I should do it in 2 functions in a different commit
same for the loop (its correct to start from 1 because 0 was checked before... using iterator is not the best solution)

I see no body has commented on llvm-dev... any other way to get more comments?

asbirlea resigned from this revision.Sep 15 2021, 12:28 PM