> Minor nit. Since this is used in an unspec_volatile, the name should be
> UNSPECV_ and defined in the unspecv enum.
Ouch. It turns out that the ARM implementation has it too so I have installed
the attached patchlet on top of the others after minimal retesting.
PR middle-end/65958
> + (unspec_volatile:PTR [(match_operand:PTR 1 "register_operand" "0")
> + (match_operand:PTR 2 "register_operand" "r")]
> +UNSPEC_PROBE_STACK_RANGE))]
Minor nit. Since this is used in an unspec_volatile, the name should be
UNSPECV_ and de
> Looks ok to me. OK /Marcus
Thanks. Testing was successful so I have installed it with a small change
(s/ARITH_BASE/ARITH_FACTOR/, it's a bit more mathematically correct).
--
Eric Botcazou
On 3 December 2015 at 12:17, Eric Botcazou 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
> 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 protectio
Sorry for the delay, very busy on other things these days...
On 16/11/15 20:00, Eric Botcazou wrote:
>> More comments inline.
>
> Revised version attached, which addresses all your comments and in
particular
> removes the
>
> +#if PROBE_INTERVAL > 4096
> +#error Cannot use indexed addressing mode
Richard,
> Revised version attached, which addresses all your comments and in
> particular removes the
>
> +#if PROBE_INTERVAL > 4096
> +#error Cannot use indexed addressing mode for stack probing
> +#endif
>
> compile-time assertion.
Any objection to me applying this revised version? I'd li
> More comments inline.
Revised version attached, which addresses all your comments and in particular
removes the
+#if PROBE_INTERVAL > 4096
+#error Cannot use indexed addressing mode for stack probing
+#endif
compile-time assertion. It generates the same code for PROBE_INTERVAL == 4096
as be
> Yes, but that's not usual, ARM and SPARC have it too, 4096 happens to be the
> limit of reg+off addressing mode on several architectures.
... not unusual...
--
Eric Botcazou
> Unless there really is common code between the two patches, this should
> be separated out into two posts, one for ARM and one for AArch64.
The ARM bits were approved by Ramana and installed right away.
> Hmm, so if PROBE_INTERVAL != 4096 we barf!
Yes, but that's not usual, ARM and SPARC have
On 06/10/15 11:11, Eric Botcazou wrote:
>> Thanks - I have no further comments on this patch. We probably need to
>> implement the same on AArch64 too in order to avoid similar problems.
>
> Here's the implementation for aarch64, very similar but simpler since there
> is
> no shortage of scratch
> Thanks - the arm backend changes are ok - but you need to wait for an
> AArch64 maintainer to review the AArch64 changes.
I installed the ARM changes long ago but the Aarch64 ones are still pending.
--
Eric Botcazou
> If I read aarch64_emit_probe_stack_range correctly, these two
> instructions are generated when (size <= PROBE_INTERVAL). If
> size <= 4 * PROBE_INTERVAL, more instructions are generated,
>
> sub x9, sp, #16384
> str xzr, [x9]
>
> sub x9, x9, #PROBE_INTERVAL
>
Hi Eric,
Thanks for the examples. I am able to map these instructions in your
examples back to RTX in your patch.
On 07/10/15 10:09, Eric Botcazou wrote:
Yes, it will generate either individual probes or a probing loop before the
frame is established when -fstack-check is passed. For aarch64,
> I assume that this patch (and arm patch) will change the instruction
> sequences in prologue. If so, do you have some examples about how
> prologue is changed with this patch? I need to adapt GDB prologue
> analyser to these new instruction sequences.
Yes, it will generate either individual pr
Hi Eric,
On 06/10/15 11:11, Eric Botcazou wrote:
Here's the implementation for aarch64, very similar but simpler since there is
no shortage of scratch registers; the only thing to note is the new blockage
pattern. This was tested on real hardware but not with Linux, instead with
Darwin (experim
On 06/10/15 11:11, Eric Botcazou wrote:
>> Thanks - I have no further comments on this patch. We probably need to
>> implement the same on AArch64 too in order to avoid similar problems.
>
> Here's the implementation for aarch64, very similar but simpler since there
> is
> no shortage of scrat
> Thanks - I have no further comments on this patch. We probably need to
> implement the same on AArch64 too in order to avoid similar problems.
Here's the implementation for aarch64, very similar but simpler since there is
no shortage of scratch registers; the only thing to note is the new block
> On targets using thumb1, I can see:
> - the new test failing (you should probably add a dg-skip or an
> effective-target directive)
OK, I have added:
/* { dg-skip-if "" { arm_thumb1 } } */
as in the ivopts-orig_biv-inc.c test.
> - gcc.dg/pr48134.c now fails, saying:
> sorry, unimplemented: -f
Hi Eric,
On 6 July 2015 at 17:46, Ramana Radhakrishnan
wrote:
>
>
> On 18/06/15 20:02, Eric Botcazou wrote:
>>> Please mark this pattern with (set_attr "type" "multiple").
>>
>> Done.
>>
>>> While I suspect that stack probing is done before any insns with invalid
>>> constants in the function, i
On 18/06/15 20:02, Eric Botcazou wrote:
>> Please mark this pattern with (set_attr "type" "multiple").
>
> Done.
>
>> While I suspect that stack probing is done before any insns with invalid
>> constants in the function, it would be better to model the length of
>> this insn so that the minipoo
> Please mark this pattern with (set_attr "type" "multiple").
Done.
> While I suspect that stack probing is done before any insns with invalid
> constants in the function, it would be better to model the length of
> this insn so that the minipool logic is not confused later in terms of
> placemen
I am not very familiar with this feature entirely so please bear with me
during review and will find some time to do some experiments with the
feature during this week, but a couple of things with respect to the
patch immediately spring to mind.
+(define_insn "probe_stack_range"
+ [(set (ma
23 matches
Mail list logo
