| Message ID | ory2evd19j.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | enable sqrt insns for cdce3.c | expand |
On Wed, 10 Mar 2021, Alexandre Oliva wrote: > > The test expects shrink-wrapping of the fsqrt call, but that will only > occur when there is a usable sqrt insn. > > Arrange for dejagnu to add the options that enable the sqrt insn, if > one is available, and to skip the test otherwise. > > > H-P, this *should* obviate the mmix-specific dg-skip-if. Unfortunately it doesn't. > Would it be > easy for you to confirm that this is the case and, if so, drop it? About as easy as for anyone (this is a compile-test), but no problem. Unfortunately I get, with your patch applied and the dg-skip-if removed: FAIL: gcc.dg/cdce3.c scan-tree-dump cdce "cdce3.c:12: [^\n\r]* function call is shrink-wrapped into error conditions." The dump files and assembly file show no obvious clues to me as to what is supposed to happen; attached. brgds, H-P ;; Function foo (foo, funcdef_no=0, decl_uid=1421, cgraph_uid=1, symbol_order=0) float foo (float x) { float _4; <bb 2> [local count: 1073741824]: _4 = sqrtf (x_2(D)); return _4; } ;; Function foo (foo, funcdef_no=0, decl_uid=1421, cgraph_uid=1, symbol_order=0) float foo (float x) { float _4; <bb 2> [local count: 1073741824]: _4 = sqrtf (x_2(D)); [tail call] return _4; } # 1 "cdce3.c" ! mmixal:= 8H LOC Data_Section .text ! mmixal:= 9H LOC 8B .p2align 2 LOC @+(4-@)&3 .global foo foo IS @ GET $1,rJ SET $3,$0 PUSHJ $2,sqrtf PUT rJ,$1 SET $0,$2 POP 1,0 .ident "GCC: (GNU) 11.0.1 20210309 (experimental) [master revision 0455cd76b687:0ed66571b1d9:63d74fed4566f1de583c368ecb9e2fc423fb1c87]" .data ! mmixal:= 8H LOC 9B
On Mar 10, 2021, Hans-Peter Nilsson <hp@bitrange.com> wrote: > On Wed, 10 Mar 2021, Alexandre Oliva wrote: >> >> The test expects shrink-wrapping of the fsqrt call, but that will only >> occur when there is a usable sqrt insn. >> >> Arrange for dejagnu to add the options that enable the sqrt insn, if >> one is available, and to skip the test otherwise. >> >> >> H-P, this *should* obviate the mmix-specific dg-skip-if. > Unfortunately it doesn't. Uhh :-( >> Would it be >> easy for you to confirm that this is the case and, if so, drop it? > About as easy as for anyone (this is a compile-test), I figured you'd have a recent toolchain around ;-) Thanks! > FAIL: gcc.dg/cdce3.c scan-tree-dump cdce "cdce3.c:12: [^\n\r]* > function call is shrink-wrapped into error conditions." How surprising! My understanding was that dg-add-options <feature> implicitly implies dg-require-effective-target <feature>. That was how I'd read et-dg-runtest. Now I see the _runtime after the check_effective_target_${target} there, and my understanding is updated, so some pending changes may need revisiting. Oh my... > The dump files and assembly file show no obvious clues to me as > to what is supposed to happen; attached. The test just should keep on not running on mmix. Without an insn for sqrt, there's no point in shrink-wrapping, and the code that performs the optimization is smart enough to realize that, so it just leaves the code alone.
On 3/9/2021 11:30 PM, Alexandre Oliva wrote: > The test expects shrink-wrapping of the fsqrt call, but that will only > occur when there is a usable sqrt insn. > > Arrange for dejagnu to add the options that enable the sqrt insn, if > one is available, and to skip the test otherwise. > > > H-P, this *should* obviate the mmix-specific dg-skip-if. Would it be > easy for you to confirm that this is the case and, if so, drop it? > > This was regstrapped on x86_64-linux-gnu, tested with a cross to a > ppc64-vxworks7r2 configured for a cpu that doesn't have fsqrt enabled, > and I'm now also regstrapping on ppc64-linux-gnu just to be sure. > Ok to install? > > > for gcc/testsuite/ChangeLog > > * gcc.dg/cdce3.c: Add sqrt insn options. OK jeff
[Revamped version of this patch, combined with others, to follow] On Mar 10, 2021, Hans-Peter Nilsson <hp@bitrange.com> wrote: > On Wed, 10 Mar 2021, Alexandre Oliva wrote: >> >> The test expects shrink-wrapping of the fsqrt call, but that will only >> occur when there is a usable sqrt insn. >> >> Arrange for dejagnu to add the options that enable the sqrt insn, if >> one is available, and to skip the test otherwise. >> >> >> H-P, this *should* obviate the mmix-specific dg-skip-if. > Unfortunately it doesn't. >> Would it be >> easy for you to confirm that this is the case and, if so, drop it? > About as easy as for anyone (this is a compile-test), but no > problem. Unfortunately I get, with your patch applied and the > dg-skip-if removed: > FAIL: gcc.dg/cdce3.c scan-tree-dump cdce "cdce3.c:12: [^\n\r]* > function call is shrink-wrapped into error conditions." Is mmix a sqrt_insn effective target? proc check_effective_target_sqrt_insn in gcc/testsuite/lib/target-supports.exp suggests it shouldn't pass, so I'm surprised it would still try to run the test despite the added /* { dg-require-effective-target sqrt_insn } */ directive. > The dump files and assembly file show no obvious clues to me as > to what is supposed to happen; attached. cdce3 is supposed to shrink-wrap the sqrtf(x) call into something like (x >= 0 ? .SQRT(x) : sqrtf(x)), where .SQRT stands for a square root instruction. Since we don't know why it still runs for you, I'm keeping the mmix explicit skip in the new version of the patch. Thanks,
On Mon, 22 Apr 2024, Alexandre Oliva wrote: > [Revamped version of this patch, combined with others, to follow] > > On Mar 10, 2021, Hans-Peter Nilsson <hp@bitrange.com> wrote: Time flies... > > On Wed, 10 Mar 2021, Alexandre Oliva wrote: > Is mmix a sqrt_insn effective target? proc > check_effective_target_sqrt_insn in > gcc/testsuite/lib/target-supports.exp suggests it shouldn't pass, so I'm > surprised it would still try to run the test despite the added > /* { dg-require-effective-target sqrt_insn } */ directive. The effective-target sqrt_insn predicate says "supports hardware square root instructions" and doesn't make a difference between sqrtdf2 (double) and sqrtsf3 (float). I'm extrapolating that the "divine meaning" of the comment is that such an instruction must be present for all supported floating-point modes for the predicate to yield true (when the predicate is correctly implemented). (We could also fix the predicate description to actually say "for all floating-point modes" and/or split the predicate into mode-specific variants, etc. ;-) MMIX has sqrtdf2 but not sqrtsf2, and the latter is what's used in cdce3.c. > cdce3 is supposed to shrink-wrap the sqrtf(x) call into something like > (x >= 0 ? .SQRT(x) : sqrtf(x)), where .SQRT stands for a square root > instruction. ...for 32-bit single floats. > Since we don't know why it still runs for you, I'm keeping the mmix > explicit skip in the new version of the patch. Thanks, that does seem like TRT. brgds, H-P
On Apr 23, 2024, Hans-Peter Nilsson <hp@bitrange.com> wrote: > (We could also fix the predicate description to actually say > "for all floating-point modes" and/or split the predicate into > mode-specific variants, etc. ;-) Yeah, I suppose that could make sense. > MMIX has sqrtdf2 but not sqrtsf2, and the latter is what's used > in cdce3.c. I see, thanks for the info.
diff --git a/gcc/testsuite/gcc.dg/cdce3.c b/gcc/testsuite/gcc.dg/cdce3.c index 601ddf055fd71..f9b3633e66a57 100644 --- a/gcc/testsuite/gcc.dg/cdce3.c +++ b/gcc/testsuite/gcc.dg/cdce3.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ /* { dg-require-effective-target hard_float } */ /* { dg-options "-O2 -fmath-errno -fdump-tree-cdce-details -fdump-tree-optimized" } */ -/* { dg-final { scan-tree-dump "cdce3.c:11: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */ +/* { dg-add-options sqrt_insn } */ +/* { dg-final { scan-tree-dump "cdce3.c:12: \[^\n\r]* function call is shrink-wrapped into error conditions\." "cdce" } } */ /* { dg-final { scan-tree-dump "sqrtf \\(\[^\n\r]*\\); \\\[tail call\\\]" "optimized" } } */ /* { dg-skip-if "doesn't have a sqrtf insn" { mmix-*-* } } */