| Message ID | orr1an97pc.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PR103097] tolerate reg-stack cross-block malformed asms | expand |
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); +}
diff --git a/gcc/reg-stack.c b/gcc/reg-stack.c index 1d9ea035cf44f..a83ee253777af 100644 --- a/gcc/reg-stack.c +++ b/gcc/reg-stack.c @@ -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); +}