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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm 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) {
----------------
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?


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

https://reviews.llvm.org/D81433