On Oct 24, 2012, at 10:47 PM, Bill Wendling wrote:
> On Oct 24, 2012, at 7:37 PM, John McCall <[email protected]> wrote:
>> On Oct 24, 2012, at 7:06 PM, Bill Wendling wrote:
>>> Yeah. I realize the text of the warning wasn't super. I'm rethinking the 
>>> whole patch though. As it turns out, people write bad ASM all the time, like
>>> 
>>> asm("foo %0", "=r" :);
>>> 
>>> I don't know if it's profitable to warn in this situation or the like. 
>> 
>> We actually reject that as syntactically ill-formed.  :)
>> 
> As we should. But it's used a lot, for instance in our tests: 
> tools/clang/test/CodeGen/mult-alt-x86.c. :-)

I don't mean that the assembly is ill-formed, I mean that the asm statement 
itself is ill-formed.  It's got commas where it's supposed to have colons, and 
it's totally missing an argument expression.  That made it somewhat challenging 
to decide what it was an example of. :)

>> I'm not totally sure what your example is getting at.  Trying to guess, if 
>> you mean that people use asm constraints that aren't consistent with how the 
>> assembly is used — e.g. if they use an =r constraint and then obviously rely 
>> on the existing value in that register in the assembly — then by all means 
>> warn about that.  It needs to be under a warning flag, of course, so that 
>> people can suppress it if they're really sure they know what they're doing;  
>> and of course it's going to be even more false-positive-prone than a normal 
>> warning, because assembly is not semantically rich, so you'd need to watch 
>> out for (say) idioms that technically read the value but actually don't 
>> depend on it in any way (like xor %0, %0).  But if you're seriously 
>> interested in putting the time into making a high-quality warning there, I 
>> think that could be very valuable for users who do rely on inline asm.
>> 
> That's basically what I'm getting at, yes. Why I reverted the patch was 
> because it needs to be thought through a bit more before I and I assume 
> others are happy with it. :)

Sure.  FWIW, I think your original warning should be a lot simpler to make 
satisfactory than the inadequate-constraint one that your later example's about.

Regardless, you'll need a warning group, and you probably ought to make two — 
one for this specific warning (-Wasm-operand-widths?), and an umbrella group 
for all warnings about asm statements (-Wasm?)

John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to