Page MenuHomePhabricator

[MachineCombiner][RISCV] Make hasReassociableSibling virtual and override it for RISCV
ClosedPublic

Authored by asi-sc on Nov 18 2022, 8:09 AM.

Details

Summary

To check reassociation correctness for RISCV, we must ensure that the root and
it's sibling have equal rounding modes (for floating point instructions).
hasReassociableSibling is a good place to make additional target-dependend
checks.

This patch allows us to enable default machine combiner mechanism to gather
reassociation candidates on RISCV.

Diff Detail

Event Timeline

asi-sc created this revision.Nov 18 2022, 8:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2022, 8:09 AM
asi-sc requested review of this revision.Nov 18 2022, 8:09 AM
asi-sc added inline comments.Nov 18 2022, 11:29 AM
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
1212–1213

Currently, hasEqualFRM returns false even if both instructions don't have FRM at all. So, to add integer instructions reassociation we:

  1. Can check that Inst and Sibling are floating-point instructions and check FRM equality only in this case Or
  2. return true from hasEqualFRM if both instructions do not have FRM operand.

I'd like to ask reviewers which way is better to follow? I'd prefer the second option.

The question is not about this review, but I'm trying to think a few steps ahead.

craig.topper accepted this revision.Nov 30 2022, 11:41 AM

LGTM

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
1212–1213

Right now hasEqualFRM returns false if either instruction doesn't have an FRM operand. Does that false case ever happen? Sounds like your option 2 here would need that to return true if the FRM operand isn't present.

This revision is now accepted and ready to land.Nov 30 2022, 11:41 AM
asi-sc added inline comments.Dec 1 2022, 4:16 AM
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
1212–1213

I spent some time thinking about this and changed my opinion. Actually, checking floating-point rounding modes of integer instructions is a strange idea. So, I'll keep the existing behavior.