Message ID | 5453514B0200007800043D79@mail.emea.novell.com |
---|---|
State | New |
Headers | show |
On 10/31/14 02:07, Jan Beulich wrote: > While it always seemed wrong to me that there's no way to avoid the > default "flags" and "fpsr" clobbers, the regression the fix for > PR/60663 introduced (see PR/63637) makes it even more desirable to have > such a mechanism: This way, at least asm()s with a single output and no > explicit clobbers can again be made subject to CSE. > > gcc: > 2014-10-31 Jan Beulich <jbeulich@suse.com> > > * config/i386/i386.c (ix86_target_string): Add > -mno-default-asm-clobbers. > (ix86_valid_target_attribute_inner_p): Handle > -m{,no-}default-asm-clobbers. > (ix86_md_asm_clobbers): Handle "inverse" clobbers. > * config/i386/i386.h (NOCC_REGNUM, NOFPSR_REGNUM): Define. > (ADDITIONAL_REGISTER_NAMES): Add "cc", "!cc", "!flags", and > "!fpsr". > * config/i386/i386.opt: Add mdefault-asm-clobbers and > mno-default-asm-clobbers. > * varasm.c (decode_reg_name_and_count): Permit negative > register numbers in ADDITIONAL_REGISTER_NAMES. > > gcc/testsuite: > 2014-10-31 Jan Beulich <jbeulich@suse.com> > > * gcc.target/i386/20060218-1.c: Adjust expected error. > * gcc.target/i386/invclbr[123].c: New. I really don't like having an option that's globally applied for this feature. THough I am OK with having a mechanism to avoid implicit clobbers on specific ASMs. Why use negative numbers for the hard register numbers? I wouldn't be at all surprised if lots of random code assumes register numbers are always positive. I don't like adding new registers with special names like !foo. Instead I think that listing "!cc" or something similar in the asm itself if it doesn't clobber the cc register would be better. Other opinions? jeff
>>> On 31.10.14 at 20:02, <law@redhat.com> wrote: > On 10/31/14 02:07, Jan Beulich wrote: >> While it always seemed wrong to me that there's no way to avoid the >> default "flags" and "fpsr" clobbers, the regression the fix for >> PR/60663 introduced (see PR/63637) makes it even more desirable to have >> such a mechanism: This way, at least asm()s with a single output and no >> explicit clobbers can again be made subject to CSE. >> >> gcc: >> 2014-10-31 Jan Beulich <jbeulich@suse.com> >> >> * config/i386/i386.c (ix86_target_string): Add >> -mno-default-asm-clobbers. >> (ix86_valid_target_attribute_inner_p): Handle >> -m{,no-}default-asm-clobbers. >> (ix86_md_asm_clobbers): Handle "inverse" clobbers. >> * config/i386/i386.h (NOCC_REGNUM, NOFPSR_REGNUM): Define. >> (ADDITIONAL_REGISTER_NAMES): Add "cc", "!cc", "!flags", and >> "!fpsr". >> * config/i386/i386.opt: Add mdefault-asm-clobbers and >> mno-default-asm-clobbers. >> * varasm.c (decode_reg_name_and_count): Permit negative >> register numbers in ADDITIONAL_REGISTER_NAMES. >> >> gcc/testsuite: >> 2014-10-31 Jan Beulich <jbeulich@suse.com> >> >> * gcc.target/i386/20060218-1.c: Adjust expected error. >> * gcc.target/i386/invclbr[123].c: New. > I really don't like having an option that's globally applied for this > feature. THough I am OK with having a mechanism to avoid implicit > clobbers on specific ASMs. Why not? That way, for projects/components knowing all their asm()s have all clobbers explicitly specified they could avoid having to touch all of them and instead just pass the new option. That said - if the option isn't being liked, I'm fine dropping it. > Why use negative numbers for the hard register numbers? I wouldn't be > at all surprised if lots of random code assumes register numbers are > always positive. I think I want through all relevant places, and fixed the one where the assumption was wrongly made. The nice thing about using negative numbers here is that there is already at least one place where negative numbers aren't valid, and we want them to not be valid there. > I don't like adding new registers with special names like !foo. Instead > I think that listing "!cc" or something similar in the asm itself if it > doesn't clobber the cc register would be better. You mean interpreting the ! in generic code? Doable, but not very nice imo considering that the default addition of clobbers is limited to very few architectures, plus dealing with this in a generic way would - afaict - make the change quite a bit more intrusive. Jan
On 11/03/14 01:11, Jan Beulich wrote: >> I really don't like having an option that's globally applied for this >> feature. THough I am OK with having a mechanism to avoid implicit >> clobbers on specific ASMs. > > Why not? That way, for projects/components knowing all their > asm()s have all clobbers explicitly specified they could avoid > having to touch all of them and instead just pass the new option. > That said - if the option isn't being liked, I'm fine dropping it. In general, I doubt projects are going to achieve and maintain that level of knowledge about their asms. It's just asking for subtle bugs in the long run. > >> Why use negative numbers for the hard register numbers? I wouldn't be >> at all surprised if lots of random code assumes register numbers are >> always positive. > > I think I want through all relevant places, and fixed the one where > the assumption was wrongly made. The nice thing about using > negative numbers here is that there is already at least one place > where negative numbers aren't valid, and we want them to not be > valid there. > >> I don't like adding new registers with special names like !foo. Instead >> I think that listing "!cc" or something similar in the asm itself if it >> doesn't clobber the cc register would be better. > > You mean interpreting the ! in generic code? Doable, but not very > nice imo considering that the default addition of clobbers is limited > to very few architectures, plus dealing with this in a generic way > would - afaict - make the change quite a bit more intrusive. yes, precisely. This is a generic concept and while more invasive, I think what we get in the end is going to ultimately be better. I also suspect other targets ought to be defining those implicit clobbers, but don't :( jeff
--- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -2664,6 +2664,7 @@ ix86_target_string (HOST_WIDE_INT isa, i { "-minline-stringops-dynamically", MASK_INLINE_STRINGOPS_DYNAMICALLY }, { "-mms-bitfields", MASK_MS_BITFIELD_LAYOUT }, { "-mno-align-stringops", MASK_NO_ALIGN_STRINGOPS }, + { "-mno-default-asm-clobbers", MASK_NO_DEFAULT_ASM_CLOBBERS }, { "-mno-fancy-math-387", MASK_NO_FANCY_MATH_387 }, { "-mno-push-args", MASK_NO_PUSH_ARGS }, { "-mno-red-zone", MASK_NO_RED_ZONE }, @@ -4649,6 +4650,10 @@ ix86_valid_target_attribute_inner_p (tre OPT_mno_align_stringops, MASK_NO_ALIGN_STRINGOPS), + IX86_ATTR_NO ("default-asm-clobbers", + OPT_mno_default_asm_clobbers, + MASK_NO_DEFAULT_ASM_CLOBBERS), + IX86_ATTR_YES ("recip", OPT_mrecip, MASK_RECIP), @@ -44165,10 +44170,31 @@ ix86_c_mode_for_suffix (char suffix) static tree ix86_md_asm_clobbers (tree, tree, tree clobbers) { - clobbers = tree_cons (NULL_TREE, build_string (5, "flags"), - clobbers); - clobbers = tree_cons (NULL_TREE, build_string (4, "fpsr"), - clobbers); + tree clobber; + bool flags_used = false, fpsr_used = false; + bool nocc_used = false, nofpsr_used = false; + + if (!TARGET_DEFAULT_ASM_CLOBBERS) + return clobbers; + + for (clobber = clobbers; clobber; clobber = TREE_CHAIN (clobber)) + switch (decode_reg_name (TREE_STRING_POINTER (TREE_VALUE (clobber)))) + { + case FLAGS_REG: flags_used = true; break; + case FPSR_REG: fpsr_used = true; break; + case NOCC_REGNUM: nocc_used = true; break; + case NOFPSR_REGNUM: nofpsr_used = true; break; + } + + if ((flags_used && nocc_used) || (fpsr_used && nofpsr_used)) + error ("conflicting clobbers in %<asm%>"); + + if (!flags_used && !nocc_used) + clobbers = tree_cons (NULL_TREE, build_string (5, "flags"), + clobbers); + if (!fpsr_used && !nofpsr_used) + clobbers = tree_cons (NULL_TREE, build_string (4, "fpsr"), + clobbers); return clobbers; } --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -1242,6 +1242,11 @@ extern const char *host_detect_local_cpu ? INVALID_REGNUM \ : REAL_PIC_OFFSET_TABLE_REGNUM) +/* Fake register numbers to be used as "inverse" asm() clobber specifiers. + Any negative numbers below the range used by decode_reg_name () will do. */ +#define NOCC_REGNUM (-127) +#define NOFPSR_REGNUM (-126) + #define GOT_SYMBOL_NAME "_GLOBAL_OFFSET_TABLE_" /* This is overridden by <cygwin.h>. */ @@ -2059,7 +2064,9 @@ do { \ { "zmm16", 53}, { "zmm17", 54}, { "zmm18", 55}, { "zmm19", 56}, \ { "zmm20", 57}, { "zmm21", 58}, { "zmm22", 59}, { "zmm23", 60}, \ { "zmm24", 61}, { "zmm25", 62}, { "zmm26", 63}, { "zmm27", 64}, \ - { "zmm28", 65}, { "zmm29", 66}, { "zmm30", 67}, { "zmm31", 68} } + { "zmm28", 65}, { "zmm29", 66}, { "zmm30", 67}, { "zmm31", 68}, \ + { "cc", FLAGS_REG }, { "!cc", NOCC_REGNUM }, \ + { "!flags", NOCC_REGNUM }, { "!fpsr", NOFPSR_REGNUM } } /* Note we are omitting these since currently I don't know how to get gcc to use these, since they want the same but different --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -287,6 +287,10 @@ Enum(pmode) String(long) Value(PMODE_DI) mcpu= Target RejectNegative Joined Undocumented Alias(mtune=) Warn(%<-mcpu=%> is deprecated; use %<-mtune=%> or %<-march=%> instead) +mdefault-asm-clobbers +Target RejectNegative Report InverseMask(NO_DEFAULT_ASM_CLOBBERS, DEFAULT_ASM_CLOBBERS) Undocumented Save +Attach compatibility clobbers (flags and fpsr) to every asm(). + mfancy-math-387 Target RejectNegative Report InverseMask(NO_FANCY_MATH_387, USE_FANCY_MATH_387) Save Generate sin, cos, sqrt for FPU @@ -355,6 +359,10 @@ Use native (MS) bitfield layout mno-align-stringops Target RejectNegative Report Mask(NO_ALIGN_STRINGOPS) Undocumented Save +mno-default-asm-clobbers +Target RejectNegative Report Mask(NO_DEFAULT_ASM_CLOBBERS) Save +Don't attach compatibility clobbers (flags and fpsr) to every asm(). + mno-fancy-math-387 Target RejectNegative Report Mask(NO_FANCY_MATH_387) Undocumented Save --- a/gcc/testsuite/gcc.target/i386/20060218-1.c +++ b/gcc/testsuite/gcc.target/i386/20060218-1.c @@ -3,6 +3,6 @@ void foo (void) { - register int cc __asm ("cc"); /* { dg-error "invalid register name" } */ + register int cc __asm ("cc"); /* { dg-error "register specified .* not general enough" } */ __asm ("" : : "r" (cc) : "cc"); } --- a/gcc/testsuite/gcc.target/i386/invclbr1.c +++ b/gcc/testsuite/gcc.target/i386/invclbr1.c @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "" } */ + +void test(void) +{ + asm ("" ::: "cc" ); + asm ("" ::: "!cc" ); + asm ("" ::: "flags" ); + asm ("" ::: "!flags" ); + asm ("" ::: "fpsr" ); + asm ("" ::: "!fpsr" ); + + asm ("" ::: "cc", "!cc" ); /* { dg-error "conflicting clobbers" } */ + asm ("" ::: "flags", "!cc" ); /* { dg-error "conflicting clobbers" } */ + asm ("" ::: "cc", "!flags" ); /* { dg-error "conflicting clobbers" } */ + asm ("" ::: "flags", "!flags" ); /* { dg-error "conflicting clobbers" } */ + asm ("" ::: "fpsr", "!fpsr" ); /* { dg-error "conflicting clobbers" } */ + + asm ("" ::: "cc", "!fpsr" ); + asm ("" ::: "flags", "!fpsr" ); + asm ("" ::: "fpsr", "!flags" ); + asm ("" ::: "fpsr", "!cc" ); +} --- a/gcc/testsuite/gcc.target/i386/invclbr2.c +++ b/gcc/testsuite/gcc.target/i386/invclbr2.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-fdump-rtl-expand" } */ + +void test(void) +{ + asm ("" ::: "!cc", "!fpsr" ); + asm ("" ::: "!fpsr", "!flags" ); +} + +/* { dg-final { scan-rtl-dump-not "clobber" "expand" } } */ +/* { dg-final { cleanup-rtl-dump "expand" } } */ --- a/gcc/testsuite/gcc.target/i386/invclbr3.c +++ b/gcc/testsuite/gcc.target/i386/invclbr3.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-fdump-rtl-expand" } */ + +void test(void) +{ + asm ("" ::: "!cc"); + asm ("" ::: "!flags"); + asm ("" ::: "!fpsr"); +} + +/* { dg-final { scan-rtl-dump-times "\\(clobber \\(reg\[^()\]* fpsr\\)\\)" 2 "expand" } } */ +/* { dg-final { scan-rtl-dump-times "\\(clobber \\(reg\[^()\]* flags\\)\\)" 1 "expand" } } */ +/* { dg-final { cleanup-rtl-dump "expand" } } */ --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -936,7 +936,7 @@ decode_reg_name_and_count (const char *a for (i = 0; i < (int) ARRAY_SIZE (table); i++) if (table[i].name[0] && ! strcmp (asmspec, table[i].name) - && reg_names[table[i].number][0]) + && (table[i].number < 0 || reg_names[table[i].number][0])) return table[i].number; } #endif /* ADDITIONAL_REGISTER_NAMES */