[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