<div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 21, 2012 at 2:02 PM, Jakob Stoklund Olesen <span dir="ltr"><<a href="mailto:stoklund@2pi.dk" target="_blank" class="cremed">stoklund@2pi.dk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">All,<br>
<br>
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.<br>
<br>
It's possible to override this behavior by setting neverHasSideEffects = 1.<br>
<br>
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.<br>

<br>
As a result, many instructions are defined without a pattern, and we often forget to set neverHasSideEffects = 1.<br>
<br>
$ grep -c UnmodeledSideEffects lib/Target/*/*GenInstrInfo.inc<br>
lib/Target/ARM/ARMGenInstrInfo.inc:727<br>
lib/Target/X86/X86GenInstrInfo.inc:920<br>
<br>
I don't think more than half of those UnmodeledSideEffects flags should be there.<br>
<br>
<br>
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.<br>
<br>
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.<br></blockquote><div><br></div><div>For what little it's worth (as I rarely touch the target td files), I like this plan. =]</div>
<div><br></div><div>I just wanted to comment on the migration bit:</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I can't just kill off the neverHasSideEffects flag, that could cause miscompilations by clearing the UnmodeledSideEffects bit on instructions that should have it.<br>
</blockquote><div><br></div><div>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.</div>
<div><br></div><div>This email (or a more clearly announce-y email) can update folks w/ out-of-tree targets.</div><div><br></div><div>Add the warning in the same go so that people immediately get complaints from tablegen until they update.</div>
<div><br></div><div>Add a big red note to the release notes for those with out-of-tree targets that don't track trunk.</div></div></div>