[PATCH] D124044: llvm-reduce: Add pass to reduce MIR instruction flags

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 27 14:26:48 PDT 2022


arsenm added a comment.

In D124044#3476361 <https://reviews.llvm.org/D124044#3476361>, @MatzeB wrote:

> Removing instruction flags seems dangerous to me:
>
> - Dropping `BundledPred`, `BundledSucc` will unbundle instructions and leave a bundle header around in the instruction stream, which seems bad.
> - I wouldn't be surprised if some targets break/segfault when `FrameSetup`/`FrameDestroy` is dropped randomly.

I forgot these were stored as part of flags. However, I think I accidentally avoided this bug. setFlags actually does take care to preserve the BundledPred/BundledSucc:

  void setFlags(unsigned flags) {
     // Filter out the automatically maintained flags.
     unsigned Mask = BundledPred | BundledSucc;
     Flags = (Flags & Mask) | (flags & ~Mask);
   }

I guess there still is potentially a bug from assuming getFlags != 0 can be reduced further, although it should at worst cause an avoidable reduction attempt.
As far as FrameSetup/FrameDestroy are concerned, it's a bug if the targets break on them. Unless there's a verifier enforced property, targets should be able to handle these unset



================
Comment at: llvm/tools/llvm-reduce/ReducerWorkItem.cpp:247
                                               /*NoImplicit=*/true);
+      DstMI->setFlags(SrcMI.getFlags());
+
----------------
MatzeB wrote:
> MatzeB wrote:
> > Nice catch.
> Feel free to just push this independently without separate review.
Also forgot about AsmPrinterFlags


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

https://reviews.llvm.org/D124044



More information about the llvm-commits mailing list