[PATCH] D78206: [Target][ARM] Make Low Overhead Loops coexist with VPT blocks

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:263
+          if (Def->getParent() == MI->getParent())
+            if (getVPTInstrPredicate(*Def) == ARMVCC::None)
+              return false;
----------------
Pierre-vh wrote:
> samparker wrote:
> > I feel like this will only be safe if we're still guaranteeing that any predicated instruction is predicated upon the VCTP. I'm concerned about some code that may look llike this:
> > 
> > 
> > ```
> > loop.ph:
> >   a = ...
> >   b = ...
> >   c = ...
> >   n = ...
> >   z = ...
> >   DLS
> > 
> > loop:
> >   VCTP
> >   VPTT z, n
> >   VSTRT
> >   VPTTTT a, b
> >   VLDRT d
> >   VLDRT e
> >   VADDT z, e, d
> >   LE loop
> > ```
> > 
> > Is it possible now to have a VCTP in the loop with nothing actually predicated upon it?
> I did this with the assumption that VPT blocks that aren't related to the VCTP would be rejected.
> 
> > Is it possible now to have a VCTP in the loop with nothing actually predicated upon it?
> 
> I think it's possible if something clears VPR.P0, or overwrites it, right after the VCTP, like so:
> ```
> VCTP
> VMSR VPR.P0 R0
> ...
> ```
> (Of course I don't know if that ever happens)
> Other instructions, such VCMP/VPT, "and" their result with VPR.P0 so those should be fine I believe.
> 
> What do you think I should do here ? Should I check that the Def is inside a VPT block that we know? (e.g. by saving all predicated Defs in a set and checking whether Def is in the set).
> I did this with the assumption that VPT blocks that aren't related to the VCTP would be rejected.
That's the idea, but I suspect that this code now no longer does. I don't mind how you go about it, but yes, somehow we need to add more information into the predicate tracking and we can't accept anything that we cannot reason about.


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

https://reviews.llvm.org/D78206