Bug 71652 - [5/6/7 Regression] ICE in in ix86_target_macros_internal, at config/i386/i386-c.c:187
Summary: [5/6/7 Regression] ICE in in ix86_target_macros_internal, at config/i386/i386...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 7.0
: P4 normal
Target Milestone: 5.5
Assignee: Not yet assigned to anyone
URL:
Keywords: error-recovery, ice-on-invalid-code
: 63902 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-06-24 17:13 UTC by Martin Liška
Modified: 2024-04-02 03:45 UTC (History)
4 users (show)

See Also:
Host:
Target: x86_64-*-*, i?86-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-07-12 00:00:00


Attachments
Candidate patch (1.10 KB, patch)
2016-07-13 13:56 UTC, Martin Liška
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2016-06-24 17:13:10 UTC
Running:

#pragma GCC push_options
#pragma GCC target ("arch=generic") // ICE

__attribute__((constructor)) void foo()
{
  asm ("");
}

#pragma GCC pop_options

int main() { return 0; }

$ gcc pr71585-ice.c
pr71585-ice.c:2:9: error: generic CPU can be used only for option("tune=") attribute
 #pragma GCC target ("arch=generic") // ICE
         ^~~
pr71585-ice.c:2:9: internal compiler error: in ix86_target_macros_internal, at config/i386/i386-c.c:187
0x8d0f27 ix86_target_macros_internal
	../../gcc/config/i386/i386-c.c:187
0x8d1ecc ix86_pragma_target_parse
	../../gcc/config/i386/i386-c.c:530
0x8ae3b1 handle_pragma_target
	../../gcc/c-family/c-pragma.c:893
0x8af160 c_invoke_pragma_handler(unsigned int)
	../../gcc/c-family/c-pragma.c:1482
0x8182e0 c_parser_pragma
	../../gcc/c/c-parser.c:10289
0x80387f c_parser_external_declaration
	../../gcc/c/c-parser.c:1537
0x80343b c_parser_translation_unit
	../../gcc/c/c-parser.c:1437
0x835a33 c_parse_file()
	../../gcc/c/c-parser.c:17985
0x8a9677 c_common_parse_file()
	../../gcc/c-family/c-opts.c:1070

GCC 5.3.1 and GCC 6.1.1 are fine.

Thanks,
Martin
Comment 1 Jakub Jelinek 2016-07-12 08:26:27 UTC
Started with r202741.
Comment 2 Martin Liška 2016-07-12 08:29:29 UTC
(In reply to Jakub Jelinek from comment #1)
> Started with r202741.

Thanks, I'm going to take a look.
Comment 3 Jakub Jelinek 2016-07-12 08:48:33 UTC
I think the bug is that
  if (!strcmp (opts->x_ix86_arch_string, "generic"))
    error ("generic CPU can be used only for %stune=%s %s",
           prefix, suffix, sw);
  else if (!strcmp (opts->x_ix86_arch_string, "intel"))
    error ("intel CPU can be used only for %stune=%s %s",
           prefix, suffix, sw);
is too late.
I'd stick it into:
  for (i = 0; i < pta_size; i++)
    if (! strcmp (opts->x_ix86_arch_string, processor_alias_table[i].name))
      {
HERE ------------>
        ix86_schedule = processor_alias_table[i].schedule;
and if emitting the error, just break, so that neither ix86_arch nor opts->x_ix86_isa_flags is affected.
Similarly, the "CPU you selected does not support x86-64 " error should not affect these.

Perhaps it would be helpful to go over the whole ix86_option_override_internal and for all errors emitted in there make sure that beyond reporting errors the function doesn't keep the options in an invalid state - changes them to some sane defaults on errors.
Some places already do that:
      error ("-mstringop-strategy=rep_8byte not supported for 32-bit code");
      opts->x_ix86_stringop_alg = no_stringop;
etc., but other spots don't.
Comment 4 Martin Liška 2016-07-13 13:52:00 UTC
Thank you very much Jakub with the suggested hint.

I applied basically what you suggested and I'm wondering whether target macro can really produce insane options that would eventually cause an ICE or another misleading error?

As I've been playing with x86 target options I came to multiple question (probably something we should fix):

1) https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html#x86-Function-Attributes section does no document all accepted values of target attribute:

fma
avx512f
avx512er
avx512cd
avx512pf
avx512dq
avx512bw
avx512vl
avx512ifma
avx512vbmi
3dnow
bmi
bmi2
lzcnt
hle
fxsr
rdseed
prfchw
adx
tbm
sha
fsgsbase
rdrnd
f16c
rtm
xsave
xsaveopt
prefetchwt1
clflushopt
xsavec
xsaves
clwb
pcommit
mwaitx

Suggestion: let's document them

2) on the other hand, ‘fused-madd’ and ‘no-fused-madd’ are document, but we reject them:
tc.cc:2:33: error: attribute(target("fused-madd")) is unknown

probably because of:
./xg++ -B. tc.cc -mfused-madd
xg++: warning: ‘-mfused-madd’ is deprecated; use ‘-ffp-contract=’ instead

Suggestion: remove the option from documentation

3) There are ISA options that are not accepted:

3dnowa
movbe
crc32
mpx
clzero
pku

Suggestion: accept them?
Comment 5 Martin Liška 2016-07-13 13:56:13 UTC
Created attachment 38886 [details]
Candidate patch
Comment 6 Jan Hubicka 2016-07-14 12:27:29 UTC
The patch looks fine to me. i would not drop the gcc_unreachable and instead just arrange the -march to be set to some other reasonable value at the time error is produced.

For all the suggestions, i think answer is yes.
Comment 7 Richard Biener 2016-08-03 12:04:03 UTC
GCC 4.9 branch is being closed
Comment 8 Martin Liška 2016-09-23 07:56:29 UTC
Author: marxin
Date: Fri Sep 23 07:55:57 2016
New Revision: 240392

URL: https://gcc.gnu.org/viewcvs?rev=240392&root=gcc&view=rev
Log:
Fix PR target/71652

	PR target/71652
	* config/i386/i386.c (ix86_option_override_internal): Change
	signature and return false when there's an error related to
	arch string.
	(release_options_strings): New function.
	(ix86_valid_target_attribute_tree): Call the function.
	* gcc.target/i386/pr71652.c: New test.
	* gcc.target/i386/pr71652-2.c: New test.
	* gcc.target/i386/pr71652-3.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr71652-2.c
    trunk/gcc/testsuite/gcc.target/i386/pr71652-3.c
    trunk/gcc/testsuite/gcc.target/i386/pr71652.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/testsuite/ChangeLog
Comment 9 Martin Liška 2016-09-23 07:59:02 UTC
Fixed on trunk.
Comment 10 Andrew Pinski 2024-04-02 03:45:34 UTC
*** Bug 63902 has been marked as a duplicate of this bug. ***