Page MenuHomePhabricator

[MachineOutliner] Make getOutliningType partially target-independent
Needs ReviewPublic

Authored by duck-37 on May 5 2022, 8:39 PM.

Details

Reviewers
paquette
Summary

The motivation behind this change is to make fixing this (and other issues like this that may crop up in the future) easier.
This is _almost_ NFC, but there are some notable changes here:

  1. isMetaInstruction() is used directly instead of checking for specific meta-instructions like IMPLICIT_DEF or KILL. This was already the case in RISCV, but other architectures still did hardcoded checks.
    • As an exception to this, CFI instructions are explicitly delegated to the target because RISCV has different handling for those.
  2. isTargetIndex() checks are replaced with an assert; none of the architectures supported actually use MO_TargetIndex at this point in time, but just removing this check may trip someone up down the line.
  3. isCFIIndex() and isFI() checks are also replaced with asserts, since these operands should not exist in any context at this stage in the pipeline.

Diff Detail

Event Timeline

duck-37 created this revision.May 5 2022, 8:39 PM
duck-37 requested review of this revision.May 5 2022, 8:39 PM
duck-37 updated this revision to Diff 427527.May 5 2022, 8:50 PM

Comment clarification and typo fix.

duck-37 updated this revision to Diff 427660.May 6 2022, 9:20 AM

Rebase and add assertions for other operands that shouldn't exist at this point.

duck-37 edited the summary of this revision. (Show Details)May 6 2022, 9:21 AM
duck-37 edited the summary of this revision. (Show Details)May 6 2022, 9:23 AM

I'm not qualified for reviewing this patch, but it makes sense to me. Thanks for doing these changes. :-)

pcwang-thead added inline comments.May 7 2022, 2:33 AM
llvm/include/llvm/CodeGen/TargetInstrInfo.h
1937

We may need to change \p MI to \p MIT or modify param name.

llvm/lib/CodeGen/TargetInstrInfo.cpp
1458

"frame indices" could be excluded since we assert them now. :-)

duck-37 updated this revision to Diff 427889.May 7 2022, 12:39 PM
duck-37 marked 2 inline comments as done.

Rebase, update a few comments.