On Oct 25, 2012, at 1:46 AM, John McCall <[email protected]> wrote: > 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?) > Yeah. That's what I was thinking. :) -bw _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
