[llvm-commits] [llvm] r45022 - in /llvm/trunk: include/llvm/Target/TargetInstrInfo.h lib/Target/Target.td utils/TableGen/CodeGenInstruction.h utils/TableGen/CodeGenTarget.cpp utils/TableGen/InstrInfoEmitter.cpp

[Bill Wendling]

Hi Chris,

> > +// M_MAY_HAVE_SIDE_EFFECTS - Set if this instruction *might* have
> > side effects,
> > +// e.g. load instructions. Note: This and M_NEVER_HAS_SIDE_EFFECTS
> > are mutually
> > +// exclusive. You can't set both! If neither flag is set, then the
> > instruction
> > +// *always* has side effects.
> > +const unsigned M_MAY_HAVE_SIDE_EFFECTS = 1 << 18;
> > +
> > +// M_NEVER_HAS_SIDE_EFFECTS - Set if this instruction *never* has
> > side effects,
> > +// e.g., xor on X86.  Note: This and M_MAY_HAVE_SIDE_EFFECTS are
> > mutually
> > +// exclusive. You can't set both! If neither flag is set, then the
> > instruction
> > +// *always* has side effects.
> > +const unsigned M_NEVER_HAS_SIDE_EFFECTS = 1 << 19;
>
> As others have pointed out, we need to be much more clear about what
> these mean.  Specifically, I'd list "NEVER" first (it is easier to
> explain). The pertinent point here is completely missing in the
> comments:  this flag is set on an instruction where there is a side
> effect that is not captured by any *operands* of the instruction or
> *other flags*.  Instructions that are "isBranch" instructions but have
> no other side effects should have M_NEVER_HAS_SIDE_EFFECTS set.  This
> flag should only be set on an instruction when *all instances* of an
> instruction of that opcode have no side effects in this way.
>
I'm confused. If we set this for an instruction where there is a side
effect that is not captured by any operands of the instr or other
flags, then it has a side effect, right? Or do you mean that we
*shouldn't* set it in this situation (which makes more sense to me)?

-bw