r225244 - Sema: analyze I,J,K,M,N,O constraints

[Chandler Carruth]

On Sat, Jan 10, 2015 at 6:25 PM, Joerg Sonnenberger <joerg at britannica.bec.de
> wrote:

> On Tue, Jan 06, 2015 at 04:26:34AM -0000, Saleem Abdulrasool wrote:
> > Author: compnerd
> > Date: Mon Jan  5 22:26:34 2015
> > New Revision: 225244
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=225244&view=rev
> > Log:
> > Sema: analyze I,J,K,M,N,O constraints
> >
> > Add additional constraint checking for target specific behaviour for
> inline
> > assembly constraints.  We would previously silently let all arguments
> through
> > for these constraints.  In cases where the constraints were violated, we
> could
> > end up failing to select instructions and triggering assertions or worse,
> > silently ignoring instructions.
>
> As discussion on IRC, I think the change is incorrect at least for 'K'.
> It makes it impossible to encapsulate the equivalent to certain MMX/SSE
> intrinsincs with always_inline functions, creating a regression for
> pixman for example. We already have logic in the backend to ensure that
> doesn't happen. In short: it is fine to verify that the constraint *can
> be* be fulfilled, but it is not acceptable to require immediates in
> cases where the inliner or GVN would be able to compute constant
> arguments later.


FWIW, I disagree.

Accepting or rejecting code should be stable across optimizations. If Clang
can't reliably fold the argument to a constant, I think it is important to
reject it.

This is not the first time that Clang has disagreed with GCC on this
specific front -- we also refuse to use the optimizer to implement
__builtin_constant_p because it would produce unpredictable results and
violates fundamental layering design decisions in LLVM and Clang.

Fortunately, pixman is the only example of code I have seen that was
regressed by this, and the code in pixman I think can even fail with GCC
under the right circumstances, so I don't see this as a problem.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150110/800c1137/attachment.html>