[PATCH] PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

[PATCH] PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

[Carl Love]

GDB maintainers:

The test powerpc-power10.exp is only expected to run properly on Power
processors that support ISA 3.1.  The test should not be run on other
processors.

This patch adds the check for ISA 3.1 to the test to disable running it
on non ISA 3.1 processors.

Please let me know if this patch is acceptable.  Thanks.

                        Carl Love

---------------------------
PowerPC: Disable test gdb.arch/powerpc-power10.exp on non  ISA 3.1 processors.

The test should only run on systems that support ISA 3.1.  Currently the
test is running on older systems generating numerous test failures.

This patch disables the test unless the system supports ISA 3.1.

The patch has been tested on Power 10 and Power 7 to verify the change
works as expected.
---
 gdb/testsuite/gdb.arch/powerpc-power10.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.arch/powerpc-power10.exp b/gdb/testsuite/gdb.arch/powerpc-power10.exp
index bc52a72d9de..f5b6f441825 100644
--- a/gdb/testsuite/gdb.arch/powerpc-power10.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-power10.exp
@@ -20,7 +20,7 @@
 standard_testfile .s
 set objfile [standard_output_file ${testfile}.o]
 
-if {![istarget "powerpc*-*-*"]} then {
+if {![istarget "powerpc*-*-*"] || [skip_power_isa_3_1_tests]} then {
     verbose "Skipping PowerPC instructions disassembly."
     return
 }
-- 
2.31.1

Re: [PATCH] PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

[Ulrich Weigand]

Carl Love <cel@us.ibm.com> wrote:

>PowerPC: Disable test gdb.arch/powerpc-power10.exp on non  ISA 3.1
>processors.

I'm wondering why this is necessary: the test never actually
*executes* the instructions, so it really only needs assembler
support for the ISA 3.1 instructions.

Also, none of the other powerpc-power[789].exp test cases
contain similar ISA guards.


Thanks,
Ulrich

Re: [PATCH] PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

[will schmidt]

On Fri, 2022-05-06 at 09:24 +0000, Ulrich Weigand wrote:
> Carl Love <cel@us.ibm.com> wrote:
> 
> > PowerPC: Disable test gdb.arch/powerpc-power10.exp on non  ISA 3.1
> > processors.
> 
> I'm wondering why this is necessary: the test never actually
> *executes* the instructions, so it really only needs assembler
> support for the ISA 3.1 instructions.

Hi,
Agree that is consistent with previous discussions we had about
guarding against ISA versions.

This failure shows up in the power7 builder results, that have just
recently started posting to the gdb-testers list. 

I suspect this is a
case where the existing hosting compiler does not understand power10 at
all, so the testcase build likely failed, and the test should have
exited before attempting to look at the results. 

The test for power10 instructions would otherwise be invalid in that
environment.   I'm not sure off top of head whether that means the test
should be marked as invalid (XFAIL) in that case, or if a newer
compiler should be added to that (pristinely antique) environment.

Thanks
-Will


> 
> Also, none of the other powerpc-power[789].exp test cases
> contain similar ISA guards.
> 
> 
> Thanks,
> Ulrich
> 

Re: [PATCH] PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

[Ulrich Weigand]

will schmidt <will_schmidt@vnet.ibm.com> wrote:

>The test for power10 instructions would otherwise be invalid in that
>environment.   I'm not sure off top of head whether that means the
test
>should be marked as invalid (XFAIL) in that case, or if a newer
>compiler should be added to that (pristinely antique) environment.

I would have expected this guard to catch that case:

if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}" object
{debug}] != "" } {

    untested "PowerPC instructions disassembly"

    return -1

}

and mark the test as UNTESTED, which seems right to me ...

Bye,
Ulrich

Re: [PATCH] PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

[Carl Love]

Ulrich:

On Fri, 2022-05-06 at 17:03 +0000, Ulrich Weigand wrote:
> will schmidt <will_schmidt@vnet.ibm.com> wrote:
> 
> > The test for power10 instructions would otherwise be invalid in
> > that
> > environment.   I'm not sure off top of head whether that means the
> test
> > should be marked as invalid (XFAIL) in that case, or if a newer
> > compiler should be added to that (pristinely antique) environment.
> 
> I would have expected this guard to catch that case:
> 
> if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}"
> object
> {debug}] != "" } {
> 
>     untested "PowerPC instructions disassembly"
> 
>     return -1
> 
> }
> 
> and mark the test as UNTESTED, which seems right to me ...

The issue is the test, powerpc-power10.s, specifies the Power 10
instructions using .long statements. 
 
        .text
        .globl func
func:
        .long 0x7c200176        /* brd r0,r1 */
        .long 0x7c2001b6        /* brh r0,r1 */
        .long 0x7c200136        /* brw r0,r1 */
        ....
        .quad 0x8004ffff0500ffff        /* xxspltidp vs0,4294967295 */
	.quad 0x8006000005000000        /* xxspltiw vs0,0 */
        .quad 0x8006000105000000        /* xxspltiw vs0,0 */
        .quad 0x8006000305000000        /* xxspltiw vs0,3 */
        .quad 0x8006000805000000        /* xxspltiw vs0,8 */


The compilation of the above assembly succeeds on systems that do not
support ISA 3.1 because the assembler does not see any opcodes.  It
can't tell what ISA the instructions the .long statements represent. 
The disassembler prints out the opcodes for patterns that match a known
opcode and the rest get printed as .long.  The issue is the
disassembled output does not match the expected values as given in
powerpc-power10.exp and thus the test generates numerous test failures.

I created a new test, disassembler_supports_isa_3_1, based on the
existing check skip_power_isa_3_1_tests.  The new check assembles the
inline pnop opcode and exits if the assembly fails.  If it does
assemble, then the check tries to disassemble the function and match
the pnop in the output to verify disassembler recognizes ISA 3.1
instsructions.  The use of the skip_power_isa_3_1_tests would probably
sufficient here since the compiler/assembler/diassembler should all be
packaged together and support the same ISA version.  But that said, in
the weird case of a system where the assembler and disassembler are for
different ISA versions, the new test is better in that it actually
tests disassembly of ISA 3.1 instructions.

With the new test, the powerpc-power10.exp test is skipped if the
system does not support disassembly of ISA 3.1 instructions.

I will post version 2 of the patch for review.

                             Carl 

[PATCH V2] PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

[Carl Love]

GDB maintainers:

The test powerpc-power10.exp is only expected to run properly on Power
processors that support assembly and disassebmly of ISA 3.1
instructions.  The issue is the test specifies the Power 10
instructions as hex values.  The compile/assembly checks in the test do
fail since the assembler doesn't know what instructions are encoded in
the hex values.

This updated patch adds an explicit check for the disassembly of ISA
3.1 instructions.  The powerpc-power10.exp test is skipped on systems
that do not support assembly and disassembly of ISA 3.1 instructions.

The patch has been tested on Power 7 and Power 10 systems.

Please let me know if this patch is acceptable.  Thanks.

                        Carl Love

-----------------------------------------------------

PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

The test should only run on systems that support disassembly of ISA 3.1
instructions.  Currently the test is running on older systems that do not
have compiler/assemble and disassembly support for ISA 3.1 instructions.
The test is generating numerous test failures on these systems.

The issue is the PowerPC ISA 3.1 instructions are all specified in hex
using .long and .quad statements.  The compiler cannot tell that the hex
patterns represent Power 10 instructions.  Thus the compilation of the test
suceeds even if the assembler doesn't support ISA 3.1.  The disassembler
matches the bits in the binary to known instruction the best that it can.
Any hex patterns that don't match are just output as .long statements.  The
result is the test, on systems that don't support ISA 3.1, fails to match
the expected Power 10 opcodes.

This patch adds a new check, disassembler_supports_isa_3_1, to determine
if the system supports disassembly of ISA 3.1 instructions.  The Power 10
instruction test is skipped if the disassembly check for ISA 3.1
instructions fails.

The patch has been tested on Power 10 and Power 7 to verify the change
works as expected.
---
 gdb/testsuite/gdb.arch/powerpc-power10.exp |  6 ++--
 gdb/testsuite/lib/gdb.exp                  | 39 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/powerpc-power10.exp b/gdb/testsuite/gdb.arch/powerpc-power10.exp
index bc52a72d9de..c0d12e06226 100644
--- a/gdb/testsuite/gdb.arch/powerpc-power10.exp
+++ b/gdb/testsuite/gdb.arch/powerpc-power10.exp
@@ -20,13 +20,13 @@
 standard_testfile .s
 set objfile [standard_output_file ${testfile}.o]
 
-if {![istarget "powerpc*-*-*"]} then {
-    verbose "Skipping PowerPC instructions disassembly."
+if {![istarget "powerpc*-*-*"] || ![disassembler_supports_isa_3_1]} then {
+    verbose "Skipping PowerPC instruction disassembly."
     return
 }
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}" object {debug}] != "" } {
-    untested "PowerPC instructions disassembly"
+    untested "PowerPC instruction disassembly"
     return -1
 }
 
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 47cb2b23676..a37f29a95c3 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -3307,6 +3307,45 @@ gdb_caching_proc skip_power_isa_3_1_tests {
     return $skip_power_isa_3_1_tests
 }
 
+gdb_caching_proc disassembler_supports_isa_3_1 {
+    global srcdir subdir gdb_prompt inferior_exited_re
+
+    set me "disassembler_supports_isa_3_1"
+    set disassembler_supports_isa_3_1 0
+
+    # Compile a test program containing ISA 3.1 instructions.
+    set src {
+	int main() {
+	asm volatile ("pnop"); // marker
+	    asm volatile ("nop");
+	    return 0;
+	}
+    }
+
+    set compile_flags ""
+    if {![gdb_simple_compile $me $src executable $compile_flags temp_obj]} {
+	# Just assume the disassembler does not support ISA 3.1 either.
+        return $disassembler_supports_isa_3_1
+    }
+
+    # No error message, compilation succeeded.  So now check if the
+    # instruction is properly disassembled as a pnop instruction.
+    clean_restart $temp_obj
+
+    # Disassemble the function.
+    gdb_test "set disassembler-options power10"
+
+    set test "disass pnop"
+    gdb_test_multiple "disass main" $test {
+	-re "\r\nDump of assembler code for function main:.*pnop.*" {
+	    set disassembler_supports_isa_3_1 1
+	}
+    }
+
+    verbose "$me:  returning $disassembler_supports_isa_3_1" 2
+    return $disassembler_supports_isa_3_1
+}
+
 # Run a test on the target to see if it supports vmx hardware.  Return 0 if so,
 # 1 if it does not.  Based on 'check_vmx_hw_available' from the GCC testsuite.
 
-- 
2.31.1

Re: [PATCH V2] PowerPC: Disable test gdb.arch/powerpc-power10.exp on non ISA 3.1 processors.

[Ulrich Weigand]

Carl Love <cel@us.ibm.com> wrote:

>This updated patch adds an explicit check for the disassembly of ISA
>3.1 instructions.  The powerpc-power10.exp test is skipped on systems
>that do not support assembly and disassembly of ISA 3.1 instructions.

This still doesn't look right to me.  The disassembler is not some
separate part of the system - the disassembler GDB uses is built
into GDB itself and compiled from sources in the bfd and opcodes
directories that are part of the binutils-gdb source tree.

This means that every GDB built from current mainline (where you would
be adding this test) must support ISA 3.1 disassembly, so the test
should be pointless.

What exactly is going wrong when running the test on a Power7
machine?  I think there's some further investigation needed ...

Bye,
Ulrich