This is an archive of the discontinued LLVM Phabricator instance.

[ARM][ReachingDefs] Remove dead code in loloops.
ClosedPublic

Authored by samparker on Nov 14 2019, 6:40 AM.

Details

Summary

Add some more helper functions to ReachingDefs to query the uses of a given MachineInstr and also to query whether two MachineInstrs use the same def of a register. For Arm, while tail-predicating, these helpers are used in the low-overhead loops to remove the dead code that calculates the number of loop iterations.

There's some duplicate code in here, from the parent patch, for including RDA in the pass.

Diff Detail

Event Timeline

samparker created this revision.Nov 14 2019, 6:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 14 2019, 6:40 AM
samparker edited the summary of this revision. (Show Details)
samparker edited the summary of this revision. (Show Details)
SjoerdMeijer added inline comments.Nov 15 2019, 4:48 AM
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
239

Just a quick question on this.
At this point, I think we have found an instruction that defines PhysReg, so I was looking into why we need to call getReachingMIDef , but perhaps you can help me, and/or place a comment if you think that is useful.

SjoerdMeijer added inline comments.Nov 15 2019, 5:03 AM
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
131

nit: just a minor simplification, don't need the else, which saves some indentation.

310–311

then we propbably also don't need this if.

315

another nit/simplification: perhaps just return early here with if (!IsTailPredicationLegal())

dmgreen added inline comments.Nov 15 2019, 5:18 AM
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
234

Does this detect other uses outside of the BB? If the value is live-out.

llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
320

Maybe make this into a lambda and do some early exits.

samparker marked 3 inline comments as done.Nov 15 2019, 6:01 AM
samparker added inline comments.
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
234

No, which is okay for my use, but I'll rename the function to be clear.

239

We're looking for uses of 'Def', so as soon as the ReachingDef of PhysReg stops being 'Def', we know there's no more users. I'll add a comment.

llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
320

I'll give it a go.

samparker updated this revision to Diff 230013.Nov 19 2019, 2:29 AM

Rebased and addressed comments.

SjoerdMeijer added inline comments.Nov 20 2019, 1:10 AM
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
351

Would MachineLoopInfo::findLoopPreheader() be useful here?

samparker updated this revision to Diff 230410.Nov 21 2019, 2:42 AM

Thanks! findLoopPreheader was exactly what I needed, so this has simplified the code in a couple of places.

SjoerdMeijer added inline comments.Nov 21 2019, 3:14 AM
llvm/test/CodeGen/Thumb2/LowOverheadLoops/mve-tail-data-types.ll
19

This doesn't seem correct to me as we haven't set up r12, the iteration count lives in r2.

samparker marked an inline comment as done.Nov 21 2019, 5:05 AM
samparker added inline comments.
llvm/test/CodeGen/Thumb2/LowOverheadLoops/mve-tail-data-types.ll
19

Good one spotting this, the logic is obviously wrong and not handling the mov r12, r2 in the loop.

samparker updated this revision to Diff 230438.Nov 21 2019, 5:38 AM

Now looking at the vctp input operand, not the output, when proving legality.

SjoerdMeijer accepted this revision.Nov 21 2019, 6:17 AM

Thanks, LGTM

This revision is now accepted and ready to land.Nov 21 2019, 6:17 AM
This revision was automatically updated to reflect the committed changes.