[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

[Evan Cheng]

On Dec 17, 2007, at 11:40 AM, Bill Wendling wrote:

> 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

I am pretty sure he meant the opposite. If M_NEVER_HAS_SIDE_EFFECTS is  
set, that means the instruction would never have any side effects that  
are not captured in the operands.

Evan

>
> 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
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits