Page MenuHomePhabricator

[IVDescriptor] Get the exact FP instruction that does not allow reordering
ClosedPublic

Authored by congzhe on Jan 24 2022, 1:19 PM.

Details

Summary

This is a bugfix in IVDescriptor.cpp.

The helper function RecurrenceDescriptor::getExactFPMathInst() is supposed to return the 1st FP instruction that does not allow reordering. However, in certain cases it does not work as expected. For instance, in the test cases added in this patch, RecurrenceDescriptor::getExactFPMathInst() returns NULL for the reduction descriptor that corresponds to the reduction PHI node. This is because when constructing the RecurrenceDescriptor, we trace the use-def chain staring from a PHI node and for each instruction in the use-def chain, its descriptor overrides the previous one . Therefore in the final RecurrenceDescriptor we constructed., we lose previous FP instructions that does not allow reordering.

For the test case added in this patch, it should not be vectorized if reordering is not allowed. However with the current trunk it is vectorized.

Diff Detail

Event Timeline

congzhe created this revision.Jan 24 2022, 1:19 PM
congzhe requested review of this revision.Jan 24 2022, 1:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2022, 1:19 PM
kmclaughlin accepted this revision.Jan 25 2022, 9:32 AM

Thank you for this fix @congzhe, LGTM! I have just added one minor comment on the new test.

llvm/test/Transforms/LoopVectorize/AArch64/strict-fadd.ll
593

Can you please add some CHECK-UNORDERED lines here? I believe for this test we should vectorise when reordering is allowed.

This revision is now accepted and ready to land.Jan 25 2022, 9:32 AM
congzhe updated this revision to Diff 402981.Jan 25 2022, 11:25 AM

Thanks for the review! I updated the patch accordingly.

kmclaughlin accepted this revision.Jan 26 2022, 9:54 AM
This revision was landed with ongoing or failed builds.Jan 26 2022, 9:35 PM
This revision was automatically updated to reflect the committed changes.