[PATCH] D79537: Add NoMerge MIFlag to avoid MIR branch folding

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

I think this looks good, but I want to find another reviewer for CodeGen. @craig.topper or @arsenm, are you OK reviewing this, or can you recommend another reviewer?



================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:109
                                         // floatint-point exceptions.
+    NoMerge      = 1 << 15,             // Instruction should not be merged
   };
----------------
arsenm wrote:
> zequanwu wrote:
> > rnk wrote:
> > > arsenm wrote:
> > > > It's not obvious to me what this means. These are also supposed to be droppable optimization flags, but it seems like you're using this for something semantically required?
> > > I don't think `BundledPred` or `FrameSetup` are droppable, IMO they are semantically significant.
> > > 
> > > The only alternative I see to this would be to encode this information in the MCInstrDesc, which would require mirroring the entire family of X86 CALL instructions, of which there are many.
> > This `NoMerge` flag is used to disable branch-fold optimization when present. What do you mean something semantically required?
> I forgot those were also here. Maybe this should be sorted up to the other mandatory flags. I'm pretty sure FrameSetup only changes debug info, so I guess that's not exactly mandatory?
> 
> I would still like a better comment here. This one just restates the name of the flag and doesn't tell me what it means by merge
I second Matt's comments on reordering the flags to group this flag away from the math flags, which are generally ignorable, and you can expand on this comment here. The notion of "merging" instructions isn't self-explanatory. Perhaps "Passes that drop source location (e.g. tail merging) info should skip this instruction." could work.


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

https://reviews.llvm.org/D79537