[PATCH] D28126: [MemDep] Handle gep with zeros for invariant.group

[Piotr Padlewski via Phabricator via llvm-commits]

Prazek marked an inline comment as done.
Prazek added inline comments.


================
Comment at: lib/Analysis/MemoryDependenceAnalysis.cpp:383-384
+      // U = bitcast Ptr  ||  U = getelementptr Ptr, 0, 0...
+      auto *BCI = dyn_cast<BitCastInst>(U);
+      auto *GEP = dyn_cast<GetElementPtrInst>(U);
+      if (BCI || (GEP && GEP->hasAllZeroIndices())) {
----------------
JDevlieghere wrote:
> Prazek wrote:
> > Prazek wrote:
> > > chandlerc wrote:
> > > > It would be nice to avoid going through the type lookup machinery twice here.
> > > Code elegance your concern here right?
> > > I can put it in ifs like
> > > 
> > >   if (isa<BitCastInst>(U)) {
> > >     TryInsertToQueue(U);
> > >     continue;
> > >   }
> > >   if (auto *GEP = dyn_cast<GetElementPtr>(U)) {
> > >     if(GEP->hasAllZeroIndices())
> > >       TryInsertToQueue(U);
> > >     continue;
> > >   }
> > > 
> > > I am not sure if it will be cleaner (specially because of nested if)
> > > 
> > >   
> > >   
> > 
> > if (auto *GEP = dyn_cast<GetElementPtr>(U); GEP && GEP->hasAllZeroIndices()) {
> >   TryInsertToQueue(U);
> >   continue;
> > }
> > 
> > This is exactly why I really would love to use C++17 in LLVM :)
> Alternatively you could switch on `U->getOpcode()` and use a regular cast. This way you have to check the type only once, at the expense of adding more clutter. Wouldn't do it here though. 
I am pretty sure that clang is smart enough to do the exact same thing (all the functions are inline so it is not that hard)


https://reviews.llvm.org/D28126