Bug 80569 - i686: "shrx" instruction generated in 16-bit mode
Summary: i686: "shrx" instruction generated in 16-bit mode
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 6.3.0
: P3 normal
Target Milestone: 5.5
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2017-04-29 22:59 UTC by Davin McCall
Modified: 2018-02-01 14:07 UTC (History)
1 user (show)

See Also:
Host:
Target: i?86-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Proposed patch (361 bytes, patch)
2017-07-22 19:53 UTC, Uroš Bizjak
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Davin McCall 2017-04-29 22:59:13 UTC
The following code, compiled with -m16 -O2 -c, fails at assembly:

--- begin ---
void load_kernel(void *setup_addr)
{
    unsigned int seg = (unsigned int)setup_addr >> 4;
    asm("movl %0, %%es" : : "r"(seg));
}
--- end ---

$ gcc -m16 -O2 -c shrxdtestcase.i 
/tmp/ccGS34WK.s: Assembler messages:
/tmp/ccGS34WK.s:11: Error: instruction `shrx' isn't supported in 16-bit mode.
Comment 1 Davin McCall 2017-04-29 22:59:56 UTC
(Prevents building Qemu).
Comment 2 Davin McCall 2017-07-22 18:30:46 UTC
Still happening in 7.1.

-march=core2 suppresses, generation of the problematic instruction happens with -march=haswell.
Comment 3 Uroš Bizjak 2017-07-22 19:53:39 UTC
Created attachment 41810 [details]
Proposed patch

Can you please test attached patch?
Comment 4 Davin McCall 2017-07-23 09:49:31 UTC
(In reply to Uroš Bizjak from comment #3)
> Can you please test attached patch?

That seems to fix the problem, yes. Thanks.
Comment 5 uros 2017-07-23 10:29:01 UTC
Author: uros
Date: Sun Jul 23 10:28:26 2017
New Revision: 250459

URL: https://gcc.gnu.org/viewcvs?rev=250459&root=gcc&view=rev
Log:
	PR target/80569
	* config/i386/i386.c (ix86_option_override_internal): Disable
	BMI, BMI2 and TBM instructions for -m16.

testsuite/ChangeLog:

	PR target/80569
	* gcc.target/i386/pr80569.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr80569.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/testsuite/ChangeLog
Comment 6 uros 2017-07-23 10:33:41 UTC
Author: uros
Date: Sun Jul 23 10:33:08 2017
New Revision: 250460

URL: https://gcc.gnu.org/viewcvs?rev=250460&root=gcc&view=rev
Log:
	PR target/80569
	* config/i386/i386.c (ix86_option_override_internal): Disable
	BMI, BMI2 and TBM instructions for -m16.

testsuite/ChangeLog:

	PR target/80569
	* gcc.target/i386/pr80569.c: New test.


Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/pr80569.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/i386/i386.c
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
Comment 7 uros 2017-07-24 18:59:53 UTC
Author: uros
Date: Mon Jul 24 18:59:21 2017
New Revision: 250479

URL: https://gcc.gnu.org/viewcvs?rev=250479&root=gcc&view=rev
Log:
	PR target/80569
	* config/i386/i386.c (ix86_option_override_internal): Disable
	BMI, BMI2 and TBM instructions for -m16.

testsuite/ChangeLog:

	PR target/80569
	* gcc.target/i386/pr80569.c: New test.


Added:
    branches/gcc-6-branch/gcc/testsuite/gcc.target/i386/pr80569.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/config/i386/i386.c
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
Comment 8 uros 2017-07-24 20:29:34 UTC
Author: uros
Date: Mon Jul 24 20:29:02 2017
New Revision: 250486

URL: https://gcc.gnu.org/viewcvs?rev=250486&root=gcc&view=rev
Log:
	PR target/80569
	* config/i386/i386.c (ix86_option_override_internal): Disable
	BMI, BMI2 and TBM instructions for -m16.

testsuite/ChangeLog:

	PR target/80569
	* gcc.target/i386/pr80569.c: New test.


Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.target/i386/pr80569.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/i386/i386.c
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
Comment 9 Uroš Bizjak 2017-07-24 20:42:21 UTC
Fixed everywhere.
Comment 10 Rainer Orth 2017-07-29 20:58:10 UTC
There are some problems with the testcase:

* Solaris/x86 with /bin/as, I get

FAIL: gcc.target/i386/pr80569.c (test for excess errors)

Excess errors:
Assembler: pr80569.c
        "/var/tmp//ccB_4KXd.s", line 2 : Illegal mnemonic
        Near line: "    .code16gcc"
        "/var/tmp//ccB_4KXd.s", line 2 : Syntax error
        Near line: "    .code16gcc"

  i386.c (x86_file_start) unconditionally emits .code16gcc without any check that
  the assembler supports it.

* on Linux/i686, I get

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/i386/pr80569.c:7:24: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  new from

  gcc is called with-m16 -march=haswell -c -m64 

  Rainer
Comment 11 ro@CeBiTec.Uni-Bielefeld.DE 2018-02-01 11:31:41 UTC
The problem still exists, and according to gcc-testresults the test
FAILs on i386-pc-solaris2.11, x86_64-pc-solaris2.11,
x86_64-apple-darwin15.6.0, i686-pc-linux-gnu, and x86_64-pc-linux-gnu.

Should I rather open a new PR for this?

	Rainer
Comment 12 ro@CeBiTec.Uni-Bielefeld.DE 2018-02-01 12:14:59 UTC
I've just checked a x86_64-apple-darwin11.4.2 build: the test PASSes for
-m64, but FAILs for -m32 with

/var/folders/zz/zyxvpxvq6csfxvn_n000087r00021y/T//cchNxmiW.s:7:no such instruction: `shrx %eax, 4(%esp),%eax'
Comment 13 Uroš Bizjak 2018-02-01 12:27:43 UTC
(In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #12)
> I've just checked a x86_64-apple-darwin11.4.2 build: the test PASSes for
> -m64, but FAILs for -m32 with
> 
> /var/folders/zz/zyxvpxvq6csfxvn_n000087r00021y/T//cchNxmiW.s:7:no such
> instruction: `shrx %eax, 4(%esp),%eax'
http://www.felixcloutier.com/x86/SARX:SHLX:SHRX.html
Comment 14 ro@CeBiTec.Uni-Bielefeld.DE 2018-02-01 13:32:09 UTC
>> /var/folders/zz/zyxvpxvq6csfxvn_n000087r00021y/T//cchNxmiW.s:7:no such
>> instruction: `shrx %eax, 4(%esp),%eax'
> http://www.felixcloutier.com/x86/SARX:SHLX:SHRX.html

Could be a bug in the old Apple as.  However, the testcase is fishy in
explicitly passing -m16.  For multilibbed x86 targets, that gets
overridden for the non-default multilibs (either with -m32 or m64).
Comment 15 H.J. Lu 2018-02-01 13:59:42 UTC
> 
> Could be a bug in the old Apple as.  However, the testcase is fishy in
> explicitly passing -m16.  For multilibbed x86 targets, that gets
> overridden for the non-default multilibs (either with -m32 or m64).

This shouldn't happen with degagnu after

http://git.savannah.gnu.org/gitweb/?p=dejagnu.git;a=commit;h=5256bd82343000c76bc0e48139003f90b6184347
Comment 16 ro@CeBiTec.Uni-Bielefeld.DE 2018-02-01 14:07:44 UTC
> This shouldn't happen with degagnu after
>
> http://git.savannah.gnu.org/gitweb/?p=dejagnu.git;a=commit;h=5256bd82343000c76bc0e48139003f90b6184347

Which would mean requiring at least DejaGnu 1.6, while install.texi
still states a minimum of 1.4.4.

The other issue (unconditionally generating .code16gcc without checking
if the assembler supports it) still stands.

	Rainer