On Mon, Jan 15, 2018 at 11:34:19AM +0000, Wilco Dijkstra wrote: > This fixes PR82964 which reports ICEs for some CONST_WIDE_INT immediates. > It turns out decimal floating point CONST_DOUBLE get changed into > CONST_WIDE_INT without checking the constraint on the operand, which > results in failures. Avoid this by only allowing SF/DF/TF mode floating > point constants in aarch64_legitimate_constant_p. A similar issue can > occur with 128-bit immediates which may be emitted even when disallowed > in aarch64_legitimate_constant_p, and the constraints in movti_aarch64 > don't match. Fix this with a new constraint and allowing valid immediates > in aarch64_legitimate_constant_p. > > Rather than allowing all 128-bit immediates and expanding in up to 8 > MOV/MOVK instructions, limit them to 4 instructions and use a literal > load for other cases. Improve the pr79041-2.c test to use a literal and > skip it for -fpic. > > This fixes all reported failures. OK for commit?
Most of this makes sense, but I don't understand this relaxation in aarch64_legitimate_constant_p > - /* Do not allow wide int constants - this requires support in movti. */ > + /* Only allow simple 128-bit immediates. */ > if (CONST_WIDE_INT_P (x)) > - return false; > + return aarch64_mov128_immediate (x); I can see why this could be correct, but it is unclear why it is neccessary to fix the bug. What goes wrong if we leave this as "return false". I think the patch looks OK otherwise, but I'd appreciate an answer on that point before you commit. Thanks, James
