On 3 December 2015 at 12:17, Eric Botcazou <ebotca...@adacore.com> wrote: >> I can understand this restriction, but... >> >> > + /* See the same assertion on PROBE_INTERVAL above. */ >> > + gcc_assert ((first % 4096) == 0); >> >> ... why isn't this a test that FIRST is aligned to PROBE_INTERVAL? > > Because that isn't guaranteed, FIRST is related to the size of the protection > area while PROBE_INTERVAL is related to the page size. > >> blank line between declarations and code. Also, can we come up with a >> suitable define for 4096 here that expresses the context and then use >> that consistently through the remainder of this function? > > OK, let's use ARITH_BASE. > >> > +(define_insn "probe_stack_range" >> > + [(set (match_operand:DI 0 "register_operand" "=r") >> > + (unspec_volatile:DI [(match_operand:DI 1 "register_operand" "0") >> > + (match_operand:DI 2 "register_operand" "r")] >> > + UNSPEC_PROBE_STACK_RANGE))] >> >> I think this should really use PTRmode, so that it's ILP32 ready (I'm >> not going to ask you to make sure that works though, since I suspect >> there are still other issues to resolve with ILP32 at this time). > > Done. Manually tested for now, I'll fully test it if approved.
Looks ok to me. OK /Marcus
