| Message ID | 20180110215348.315-2-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10.01.2018 22:53, David Hildenbrand wrote: > While new compilers like > s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1) > Complain, that R1 is missing, old compilers like > gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16) > will complain that R1 is not valid. > > According to the architecture, R1 is valid but ignored. No idea why this > was changed (in a way that programs no longer compile). What a pity :-( > Anyhow, let's just specify the instruction explicitly. > > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > s390x/intercept.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/s390x/intercept.c b/s390x/intercept.c > index 59e5fca..99dde0d 100644 > --- a/s390x/intercept.c > +++ b/s390x/intercept.c > @@ -139,7 +139,7 @@ static void test_testblock(void) > > asm volatile ( > " lghi %%r0,0\n" > - " tb %1\n" > + " .insn rre,0xb22c0000,0,%1\n" > " ipm %0\n" > " srl %0,28\n" > : "=d" (cc) > @@ -150,12 +150,12 @@ static void test_testblock(void) > > expect_pgm_int(); > low_prot_enable(); > - asm volatile (" tb %0 " : : "r"(4096)); > + asm volatile (" .insn rre,0xb22c0000,0,%0\n" : : "r"(4096)); > low_prot_disable(); > check_pgm_int_code(PGM_INT_CODE_PROTECTION); > > expect_pgm_int(); > - asm volatile (" tb %0 " : : "r"(-4096)); > + asm volatile (" .insn rre,0xb22c0000,0,%0\n" : : "r"(-4096)); > check_pgm_int_code(PGM_INT_CODE_ADDRESSING); > } > > Reviewed-by: Thomas Huth <thuth@redhat.com>
On 01/11/2018 09:51 AM, Thomas Huth wrote: > On 10.01.2018 22:53, David Hildenbrand wrote: >> While new compilers like >> s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1) >> Complain, that R1 is missing, old compilers like >> gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16) >> will complain that R1 is not valid. >> >> According to the architecture, R1 is valid but ignored. No idea why this >> was changed (in a way that programs no longer compile). > > What a pity :-( IIR, this was a bugfix. gcc is also used for TPF and they actually use TB and the old binutils support for TB was architecturally wrong. Since TB is really of no use for Linux this should not matter that much apart from the testcases to test the KVM implementation.
On 11.01.2018 16:56, Christian Borntraeger wrote: > > > On 01/11/2018 09:51 AM, Thomas Huth wrote: >> On 10.01.2018 22:53, David Hildenbrand wrote: >>> While new compilers like >>> s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1) >>> Complain, that R1 is missing, old compilers like >>> gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16) >>> will complain that R1 is not valid. >>> >>> According to the architecture, R1 is valid but ignored. No idea why this >>> was changed (in a way that programs no longer compile). >> >> What a pity :-( > > IIR, this was a bugfix. gcc is also used for TPF and they actually use TB and > the old binutils support for TB was architecturally wrong. Since TB is really > of no use for Linux this should not matter that much apart from the testcases > to test the KVM implementation. > Well they managed to break code, so this was indeed a bad decision.
On 01/11/2018 05:00 PM, David Hildenbrand wrote: > On 11.01.2018 16:56, Christian Borntraeger wrote: >> >> >> On 01/11/2018 09:51 AM, Thomas Huth wrote: >>> On 10.01.2018 22:53, David Hildenbrand wrote: >>>> While new compilers like >>>> s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1) >>>> Complain, that R1 is missing, old compilers like >>>> gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16) >>>> will complain that R1 is not valid. >>>> >>>> According to the architecture, R1 is valid but ignored. No idea why this >>>> was changed (in a way that programs no longer compile). >>> >>> What a pity :-( >> >> IIR, this was a bugfix. gcc is also used for TPF and they actually use TB and >> the old binutils support for TB was architecturally wrong. Since TB is really >> of no use for Linux this should not matter that much apart from the testcases >> to test the KVM implementation. >> > > Well they managed to break code, so this was indeed a bad decision. With that Binutils change we only report an error for code which was broken anyway. When writing this testcase the right thing would have been to report a Binutils bug instead of writing a testcase which uses an instruction which isn't part of the POP. -Andreas-
>> Well they managed to break code, so this was indeed a bad decision. > > With that Binutils change we only report an error for code which was broken anyway. Well it worked, so it wasn't broken. I would just have liked this instruction to be fixed in a backwards compatible way (e.g. warning - or is it just a warning and my gcc flags treat them as errors?). > > When writing this testcase the right thing would have been to report a Binutils bug instead of > writing a testcase which uses an instruction which isn't part of the POP. Huh? PoP - "Control Instructions" - 10-176 - "TEST BLOCK". Even can find it in the Pop from 2004. But anyhow, we have this fixed now. Was just wondering, why our _working_ code suddenly broke (and now we have to use .insn to make it compile with GCC in general). Thanks. > > -Andreas- >
On 01/11/2018 08:11 PM, David Hildenbrand wrote: > >>> Well they managed to break code, so this was indeed a bad decision. >> >> With that Binutils change we only report an error for code which was broken anyway. > > Well it worked, so it wasn't broken. I would just have liked this > instruction to be fixed in a backwards compatible way (e.g. warning - or > is it just a warning and my gcc flags treat them as errors?). One operand was missing. The ommitted operand was just 0 then. The instruction never did what the user expected. That nobody complained about using it that way doesn't mean it was correct. >> When writing this testcase the right thing would have been to report a Binutils bug instead of >> writing a testcase which uses an instruction which isn't part of the POP. > > Huh? > > PoP - "Control Instructions" - 10-176 - "TEST BLOCK". Even can find it > in the Pop from 2004. It wasn't in the POP the way the testcase used it - with just one operand. It was a bug in Binutils since the very beginning probably. > But anyhow, we have this fixed now. Was just wondering, why our > _working_ code suddenly broke (and now we have to use .insn to make it > compile with GCC in general) -Andreas-
diff --git a/s390x/intercept.c b/s390x/intercept.c index 59e5fca..99dde0d 100644 --- a/s390x/intercept.c +++ b/s390x/intercept.c @@ -139,7 +139,7 @@ static void test_testblock(void) asm volatile ( " lghi %%r0,0\n" - " tb %1\n" + " .insn rre,0xb22c0000,0,%1\n" " ipm %0\n" " srl %0,28\n" : "=d" (cc) @@ -150,12 +150,12 @@ static void test_testblock(void) expect_pgm_int(); low_prot_enable(); - asm volatile (" tb %0 " : : "r"(4096)); + asm volatile (" .insn rre,0xb22c0000,0,%0\n" : : "r"(4096)); low_prot_disable(); check_pgm_int_code(PGM_INT_CODE_PROTECTION); expect_pgm_int(); - asm volatile (" tb %0 " : : "r"(-4096)); + asm volatile (" .insn rre,0xb22c0000,0,%0\n" : : "r"(-4096)); check_pgm_int_code(PGM_INT_CODE_ADDRESSING); }
While new compilers like s390x-linux-gnu-gcc (GCC) 7.2.1 20170915 (Red Hat Cross 7.2.1-1) Complain, that R1 is missing, old compilers like gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16) will complain that R1 is not valid. According to the architecture, R1 is valid but ignored. No idea why this was changed (in a way that programs no longer compile). Anyhow, let's just specify the instruction explicitly. Signed-off-by: David Hildenbrand <david@redhat.com> --- s390x/intercept.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)