| Message ID | 20240904220707.3900-1-palmer@rivosinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Make the setCC/REE tests robust to instruction selection | expand |
On 9/4/24 4:07 PM, Palmer Dabbelt wrote: > These tests were checking that the output of the setCC instruction was bit > flipped, but it looks like they're really designed to test that > redundant sign extension elimination fires on conditionals from function > inputs. Jeff just posed a patch to clean this code up with trips up on > the arbitrary xori/snez instruction selection decision changing, so > let's just robustify the tests. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/sge.c: Adjust regex to match the input. > * gcc.target/riscv/sgeu.c: Likewise. > * gcc.target/riscv/sle.c: Likewise. > * gcc.target/riscv/sleu.c: Likewise. Works for me. Didn't see worth much effort here. Based on the history of those tests their main purpose is to ensure we don't have an extension after the sCC. > --- > I haven't tested this, but it should be indepndent from Jeff's. Independent, but it would eliminate the need for my twiddle to these tests. I don't much care either way other than making sure they stay in a PASS state ;-) jeff
On 9/4/24 4:07 PM, Palmer Dabbelt wrote: > These tests were checking that the output of the setCC instruction was bit > flipped, but it looks like they're really designed to test that > redundant sign extension elimination fires on conditionals from function > inputs. Jeff just posed a patch to clean this code up with trips up on > the arbitrary xori/snez instruction selection decision changing, so > let's just robustify the tests. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/sge.c: Adjust regex to match the input. > * gcc.target/riscv/sgeu.c: Likewise. > * gcc.target/riscv/sle.c: Likewise. > * gcc.target/riscv/sleu.c: Likewise. > --- > I haven't tested this, but it should be indepndent from Jeff's. pre-commit CI was happy with this. So I went ahead and pushed. My patch is going to need a v2 anyway and having this installed simplifies my v2 ;-) jeff
On Wed, 04 Sep 2024 15:20:45 PDT (-0700), jeffreyalaw@gmail.com wrote: > > > On 9/4/24 4:07 PM, Palmer Dabbelt wrote: >> These tests were checking that the output of the setCC instruction was bit >> flipped, but it looks like they're really designed to test that >> redundant sign extension elimination fires on conditionals from function >> inputs. Jeff just posed a patch to clean this code up with trips up on >> the arbitrary xori/snez instruction selection decision changing, so >> let's just robustify the tests. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/sge.c: Adjust regex to match the input. >> * gcc.target/riscv/sgeu.c: Likewise. >> * gcc.target/riscv/sle.c: Likewise. >> * gcc.target/riscv/sleu.c: Likewise. > Works for me. Didn't see worth much effort here. Based on the history > of those tests their main purpose is to ensure we don't have an > extension after the sCC. Ah, OK, I guess I misunderstood then -- we'd had a bunch of issues overly sign extending inputs, so I figured that's what this was trying to do. I think the output side is still covered by the scan-assembler-not for W ops, so we should be safe there. > >> --- >> I haven't tested this, but it should be indepndent from Jeff's. > Independent, but it would eliminate the need for my twiddle to these > tests. I don't much care either way other than making sure they stay > in a PASS state ;-) Ya, sorry, I guess "independent" is really the wrong word there. I was trying to say we could merge this and it'd pass before/after your change. > > jeff
diff --git a/gcc/testsuite/gcc.target/riscv/sge.c b/gcc/testsuite/gcc.target/riscv/sge.c index 5f7e7ae82db..70f934c4d0f 100644 --- a/gcc/testsuite/gcc.target/riscv/sge.c +++ b/gcc/testsuite/gcc.target/riscv/sge.c @@ -8,5 +8,5 @@ sge (int x, int y) return x >= y; } -/* { dg-final { scan-assembler "\\sxori\\sa0,a0,1\n\\sret\n" } } */ +/* { dg-final { scan-assembler "slt\\sa0,a0,a1" } } */ /* { dg-final { scan-assembler-not "andi|sext\\.w" } } */ diff --git a/gcc/testsuite/gcc.target/riscv/sgeu.c b/gcc/testsuite/gcc.target/riscv/sgeu.c index 234b9aa52bd..0ff21cfe5e0 100644 --- a/gcc/testsuite/gcc.target/riscv/sgeu.c +++ b/gcc/testsuite/gcc.target/riscv/sgeu.c @@ -8,5 +8,5 @@ sgeu (unsigned int x, unsigned int y) return x >= y; } -/* { dg-final { scan-assembler "\\sxori\\sa0,a0,1\n\\sret\n" } } */ +/* { dg-final { scan-assembler "sltu\\sa0,a0,a1" } } */ /* { dg-final { scan-assembler-not "andi|sext\\.w" } } */ diff --git a/gcc/testsuite/gcc.target/riscv/sle.c b/gcc/testsuite/gcc.target/riscv/sle.c index 3259c191598..770840d0564 100644 --- a/gcc/testsuite/gcc.target/riscv/sle.c +++ b/gcc/testsuite/gcc.target/riscv/sle.c @@ -8,5 +8,5 @@ sle (int x, int y) return x <= y; } -/* { dg-final { scan-assembler "\\sxori\\sa0,a0,1\n\\sret\n" } } */ +/* { dg-final { scan-assembler "sgt\\sa0,a0,a1" } } */ /* { dg-final { scan-assembler-not "andi|sext\\.w" } } */ diff --git a/gcc/testsuite/gcc.target/riscv/sleu.c b/gcc/testsuite/gcc.target/riscv/sleu.c index 301b8c32eb7..ae00ccc2067 100644 --- a/gcc/testsuite/gcc.target/riscv/sleu.c +++ b/gcc/testsuite/gcc.target/riscv/sleu.c @@ -8,5 +8,5 @@ sleu (unsigned int x, unsigned int y) return x <= y; } -/* { dg-final { scan-assembler "\\sxori\\sa0,a0,1\n\\sret\n" } } */ +/* { dg-final { scan-assembler "sgtu\\sa0,a0,a1"} } */ /* { dg-final { scan-assembler-not "andi|sext\\.w" } } */