[PR103097] tolerate reg-stack cross-block malformed asms
Commit Message
The testcase shows malformed asms in one block confuse reg-stack logic
in another block. Moving the resetting of any_malformed_asm to the
end of the pass enables it to take effect throughout the affected
function.
Regstrapped on x86_64-linux-gnu. Ok to install?
for gcc/ChangeLog
PR target/103097
* reg-stack.c (convert_regs_1): Move any_malformed_asm
resetting...
(reg_to_stack): ... here.
for gcc/testsuite/ChangeLog
PR target/103097
* gcc.target/i386/pr103097.c: New.
---
gcc/reg-stack.c | 3 +--
gcc/testsuite/gcc.target/i386/pr103097.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/pr103097.c
Comments
On 12/7/2021 7:00 PM, Alexandre Oliva via Gcc-patches wrote:
> The testcase shows malformed asms in one block confuse reg-stack logic
> in another block. Moving the resetting of any_malformed_asm to the
> end of the pass enables it to take effect throughout the affected
> function.
>
> Regstrapped on x86_64-linux-gnu. Ok to install?
>
>
> for gcc/ChangeLog
>
> PR target/103097
> * reg-stack.c (convert_regs_1): Move any_malformed_asm
> resetting...
> (reg_to_stack): ... here.
>
> for gcc/testsuite/ChangeLog
>
> PR target/103097
> * gcc.target/i386/pr103097.c: New.
So it's "stickier" after your change. ie, instead of indicating if
there was a malformed insn in a block, it's did we find a malformed insn
anywhere. Which implies the comment before the declaration of
any_malformed_asm needs a trivial update since it stated "malformed
insns in a block".
OK with the trivial comment update.
jeff
On Dec 8, 2021, Jeff Law <jeffreyalaw@gmail.com> wrote:
> On 12/7/2021 7:00 PM, Alexandre Oliva via Gcc-patches wrote:
>> PR target/103097
>> * reg-stack.c (convert_regs_1): Move any_malformed_asm
>> resetting...
>> (reg_to_stack): ... here.
> So it's "stickier" after your change. ie, instead of indicating if
> there was a malformed insn in a block, it's did we find a malformed
> insn anywhere.
Yeah, anywhere in the same function.
> Which implies the comment before the declaration of any_malformed_asm
> needs a trivial update since it stated "malformed insns in a block".
(-: But, but... we still find it in a block! ;-D
> OK with the trivial comment update.
Thanks, I'm making it s/block/function/.
Here's what I'm installing momentarily.
[PR103097] tolerate reg-stack cross-block malformed asms
The testcase shows malformed asms in one block confuse reg-stack logic
in another block. Moving the resetting of any_malformed_asm to the
end of the pass enables it to take effect throughout the affected
function.
for gcc/ChangeLog
PR target/103097
* reg-stack.c (convert_regs_1): Move any_malformed_asm
resetting...
(reg_to_stack): ... here.
for gcc/testsuite/ChangeLog
PR target/103097
* gcc.target/i386/pr103097.c: New.
---
gcc/reg-stack.c | 5 ++---
gcc/testsuite/gcc.target/i386/pr103097.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/pr103097.c
diff --git a/gcc/reg-stack.c b/gcc/reg-stack.c
index 1d9ea035cf44f..cc369f0b635a0 100644
--- a/gcc/reg-stack.c
+++ b/gcc/reg-stack.c
@@ -453,7 +453,7 @@ get_true_reg (rtx *pat)
}
}
-/* Set if we find any malformed asms in a block. */
+/* Set if we find any malformed asms in a function. */
static bool any_malformed_asm;
/* There are many rules that an asm statement for stack-like regs must
@@ -3014,8 +3014,6 @@ convert_regs_1 (basic_block block)
bool cfg_altered = false;
int debug_insns_with_starting_stack = 0;
- any_malformed_asm = false;
-
/* Choose an initial stack layout, if one hasn't already been chosen. */
if (bi->stack_in.top == -2)
{
@@ -3385,6 +3383,7 @@ reg_to_stack (void)
0, sizeof (char) * (max_uid + 1));
convert_regs ();
+ any_malformed_asm = false;
free_aux_for_blocks ();
return true;
diff --git a/gcc/testsuite/gcc.target/i386/pr103097.c b/gcc/testsuite/gcc.target/i386/pr103097.c
new file mode 100644
index 0000000000000..2b7a307deec9a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr103097.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fharden-conditional-branches" } */
+
+/* This is a slightly simplified version of
+ gcc.target/s390/vector/long-double-asm-earlyclobber.c. On x86, the f
+ constraints in asm statements imposes some requirements that the testcase
+ doesn't meet. What's unusual is that -fharden-conditional-branches extends
+ the effects of the malformed asm onto a different basic blocks, which
+ reg-stack did not expect. */
+
+#include <assert.h>
+#include <stdint.h>
+
+void
+f (void)
+{
+ long double res, x = 0;
+ asm("" : "=f"(res) /* { dg-error "must specify a single register" } */
+ : "0"(x));
+ assert (res == x);
+}
+
+void
+g (void)
+{
+ long double res, x = 0;
+ asm("" : "=g"(res) /* this is ok. */
+ : "0"(x));
+ assert (res == x);
+}
@@ -3014,8 +3014,6 @@ convert_regs_1 (basic_block block)
bool cfg_altered = false;
int debug_insns_with_starting_stack = 0;
- any_malformed_asm = false;
-
/* Choose an initial stack layout, if one hasn't already been chosen. */
if (bi->stack_in.top == -2)
{
@@ -3385,6 +3383,7 @@ reg_to_stack (void)
0, sizeof (char) * (max_uid + 1));
convert_regs ();
+ any_malformed_asm = false;
free_aux_for_blocks ();
return true;
new file mode 100644
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-O1 -fharden-conditional-branches" } */
+
+/* This is a slightly simplified version of
+ gcc.target/s390/vector/long-double-asm-earlyclobber.c. On x86, the f
+ constraints in asm statements imposes some requirements that the testcase
+ doesn't meet. What's unusual is that -fharden-conditional-branches extends
+ the effects of the malformed asm onto a different basic blocks, which
+ reg-stack did not expect. */
+
+#include <assert.h>
+#include <stdint.h>
+
+void
+f (void)
+{
+ long double res, x = 0;
+ asm("" : "=f"(res) /* { dg-error "must specify a single register" } */
+ : "0"(x));
+ assert (res == x);
+}
+
+void
+g (void)
+{
+ long double res, x = 0;
+ asm("" : "=g"(res) /* this is ok. */
+ : "0"(x));
+ assert (res == x);
+}