[PATCH] D81433: AMDGPU: Restrict the number of instructions to scan for getPointerDependencyFrom

[Changpeng Fang via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAnnotateUniformValues.cpp:109
+  // Maximum number of instructions to search in a block (default 100).
+  auto Limit = MDR->getDefaultBlockScanLimit();
   for (auto &BB : Checklist) {
----------------
arsenm wrote:
> cfang wrote:
> > arsenm wrote:
> > > I don't see how this changes anything. This is what the default null case does?
> > > 
> > >   unsigned DefaultLimit = getDefaultBlockScanLimit();
> > >   if (!Limit)
> > >     Limit = &DefaultLimit;
> > > 
> > Ooh, the change is even not correct due to the weird design of the Limit usage:
> >     --*Limit;
> >     if (!*Limit)
> >       return MemDepResult::getUnknown();
> > 
> > This will usually causes trouble if you don't use default nullptr. You will have to set the Limit before each call.
> > 
> > To say that, the use at DeadStoreElimination.cpp is also not correct.
> Seems like you should fix that instead of working around it here, or make it a mandatory argument?
We should fix DeadStoreElimination case anyway.

And it seems DeadStoreElimination is the only case using explicit argument. However I am using
an argument is still useful which allows the user to specify the limit instead of using the default. 
We may just pass by value with a negative meaning default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81433/new/

https://reviews.llvm.org/D81433