[LLVMdev] Let's get rid of neverHasSideEffects

[Chris Lattner]

On Aug 21, 2012, at 3:45 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
>> 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.

Ok, I agree with that.  I think it's crazy that we still can't write a pattern for copy/move instructions, so we have to smatter attributes on them.  What are the other cases that we can't write a pattern for them?

I agree that it is sometimes convenient to write Pat<> patterns instead of putting a pattern on an instruction: why not make tblgen also infer predicates from Pat patterns that expand to a single instruction?

>> 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 ;)

It's also a serious tblgen deficiency that we can't write patterns for these.  Look at the terrible code in X86ISelDAGToDAG.cpp that is required to match these.

> 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.

Personally, I don't like the direction of making everything be redundantly specified with "let" clauses and in the patterns.  I agree that it is a problem that we're not inferring from Pat<> patterns, and that not all instructions are expressible with patterns, but I'd rather we solve *those* problems than throw out inference.

-Chris