[LLVMdev] Let's get rid of neverHasSideEffects

[Chandler Carruth]

On Tue, 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.
>
> It's possible to override this behavior by setting neverHasSideEffects = 1.
>
> It was originally the intention that most instructions have patterns, but
> that's not the way it worked out. It is often more convenient to use def :
> Pat<>, and sometimes custom instruction selection is required.
>
> As a result, many instructions are defined without a pattern, and we often
> forget to set neverHasSideEffects = 1.
>
> $ grep -c UnmodeledSideEffects lib/Target/*/*GenInstrInfo.inc
> lib/Target/ARM/ARMGenInstrInfo.inc:727
> lib/Target/X86/X86GenInstrInfo.inc:920
>
> I don't think more than half of those UnmodeledSideEffects flags should be
> there.
>
>
> I want to stop inferring instruction properties from patterns in TableGen.
> It has become very hard to read instruction definitions when some
> properties are inferred, but only half the time.
>
> We can still use any patterns to emit TableGen warnings. If an instruction
> has a sideeffecting pattern, but hasSideEffects isn't set, TableGen should
> complain.
>

For what little it's worth (as I rarely touch the target td files), I like
this plan. =]

I just wanted to comment on the migration bit:


> I can't just kill off the neverHasSideEffects flag, that could cause
> miscompilations by clearing the UnmodeledSideEffects bit on instructions
> that should have it.
>

Why not mass-update all the td files in the tree to explicitly set the flag
exactly as it is being inferred today, then kill the inference. The
behavior is the same, but now explicit, and we can go through and remove
all the places that make no sense.

This email (or a more clearly announce-y email) can update folks w/
out-of-tree targets.

Add the warning in the same go so that people immediately get complaints
from tablegen until they update.

Add a big red note to the release notes for those with out-of-tree targets
that don't track trunk.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120821/333aecd6/attachment.html>