[LLVMdev] Let's get rid of neverHasSideEffects

[Jakob Stoklund Olesen]

On Aug 21, 2012, at 3:02 PM, Chris Lattner <clattner at apple.com> wrote:

> 
> On Aug 21, 2012, at 2:02 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
> 
>> All,
>> 
>> TableGen likes to infer the MCID::UnmodeledSideEffects flag from an instruction's pattern. When an instruction doesn't have a pattern, it is assumed to have side effects.
> 
> Hi Jakob,
> 
> I don't understand what you're saying.  Are you proposing that all properties (may load, store, side effects) be explicitly added to all instructions, and the pattern only be used to produce warnings?

Yes.

The side effect inference is worse than the load/store inference, but features like this work best when they work all the time. If all instructions had patterns, and we could accurately infer properties, it wouldn't be a problem.

> Won't this hugely bloat the .td files?

Not really, TableGen has a fairly convenient syntax for bulk flagging:

let mayLoad = 1 in {
let Defs = [AL,EFLAGS,AX], Uses = [AX] in
def DIV8m  : I<0xF6, MRM6m, (outs), (ins i8mem:$src),   // AX/[mem8] = AL,AH
               "div{b}\t$src", [], IIC_DIV8_MEM>;
let Defs = [AX,DX,EFLAGS], Uses = [AX,DX] in
def DIV16m : I<0xF7, MRM6m, (outs), (ins i16mem:$src),  // DX:AX/[mem16] = AX,DX
               "div{w}\t$src", [], IIC_DIV16>, OpSize;
let Defs = [EAX,EDX,EFLAGS], Uses = [EAX,EDX] in    // EDX:EAX/[mem32] = EAX,EDX
def DIV32m : I<0xF7, MRM6m, (outs), (ins i32mem:$src),
               "div{l}\t$src", [], IIC_DIV32>;
// RDX:RAX/[mem64] = RAX,RDX
let Defs = [RAX,RDX,EFLAGS], Uses = [RAX,RDX] in
def DIV64m : RI<0xF7, MRM6m, (outs), (ins i64mem:$src),
                "div{q}\t$src", [], IIC_DIV64>;
}

It is a complete coincidence that these instructions happen to be missing neverHasSideEffects = 1 ;)

Often, you can set the mayLoad flag as part of a multiclass when you define the *rm variant. X86 is the worst case because so many instructions can fold loads and stores. You can handle most of them by setting the mayLoad and mayStore flags in the big ArithBinOp multiclasses.

You can see the problem in the big ArithBinOp_RF multiclass in X86InstrArithmetic.td. It's not obvious at a glance which of those instructions have patterns, and as a result we forgot to set neverHasSideEffects on the *_REV and *8i8 variants. I don't think it would detract to add "let mayLoad = 1 in {…}" around the *rm groups etc.

Or you could set the flags on the various BinOpRMW super classes.

/jakob