On Thu, May 1, 2014 at 6:42 AM, Wei Mi <w...@google.com> wrote: > Ping. Is pr58066-3.patch or pr58066-4.patch ok for trunk?
None of these patches have correct ChangeLog entries. Please follow the rules, outlined in http://gcc.gnu.org/contribute.html (Submitting Patches section), otherwise your patches will be simply ignored. >>> I attached the patch which combined your two patches and the fix in >>> legitimize_tls_address. I tried pr58066.c and c.i in ia32/x32/x86_64, >>> the code looked fine. Do you think it is ok? >>> >>> Thanks, >>> Wei. >> >> Either pr58066-3.patch or pr58066-4.patch looks good to me. pr58066-4 patch is definitely not OK. I wonder, how it works at all, since you can't split the insn to the same pattern. The generic code detects this condition and forces ICE (IIRC: this is the reason for UNSPEC_DIV_ALREADY_SPLIT tag in divmod<mode4_1>). >From pr58066-3 patch: -;; Local dynamic of a single variable is a lose. Show combine how -;; to convert that back to global dynamic. - -(define_insn_and_split "*tls_local_dynamic_32_once" - [(set (match_operand:SI 0 "register_operand" "=a") - (plus:SI - (unspec:SI [(match_operand:SI 1 "register_operand" "b") - (match_operand 2 "constant_call_address_operand" "z")] - UNSPEC_TLS_LD_BASE) - (const:SI (unspec:SI - [(match_operand 3 "tls_symbolic_operand")] - UNSPEC_DTPOFF)))) - (clobber (match_scratch:SI 4 "=d")) - (clobber (match_scratch:SI 5 "=c")) - (clobber (reg:CC FLAGS_REG))] - "" - "#" - "" - [(parallel - [(set (match_dup 0) - (unspec:SI [(match_dup 1) (match_dup 3) (match_dup 2)] - UNSPEC_TLS_GD)) - (clobber (match_dup 4)) - (clobber (match_dup 5)) - (clobber (reg:CC FLAGS_REG))])]) Why did you remove this splitter? Please do not write: +{ + ix86_tls_descriptor_calls_expanded_in_cfun = true; +}) but use a short form: + "ix86_tls_descriptor_calls_expanded_in_cfun = true;") Please also add a testcase (from one of the previous mails): --- testsuite/gcc.dg/pr58066.c (revision 0) +++ testsuite/gcc.dg/pr58066.c (revision 0) Put this test to gcc.target/i386 directory ... @@ -0,0 +1,18 @@ +/* { dg-do compile { target {{ i?86-*-* x86_64-*-* } && { ! ia32 } } } } */ ... to avoid target selector. +/* { dg-options "-fPIC -O2" } */ + +/* Check whether the stack frame starting addresses of tls expanded calls + in foo and goo are 16bytes aligned. */ +static __thread char ccc1; +void* foo() +{ + return &ccc1; +} + +__thread char ccc2; +void* goo() +{ + return &ccc2; +} + +/* { dg-final { scan-assembler-times ".cfi_def_cfa_offset 16" 2 } } */ Please repost the complete patch with a proper ChangeLog. Uros.
