[Bug target/54445] New: TLS array lookup with negative constant is not combined into a single instruction

[Bug target/54445] New: TLS array lookup with negative constant is not combined into a single instruction

[adam at consulting dot net.nz]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

             Bug #: 54445
           Summary: TLS array lookup with negative constant is not
                    combined into a single instruction
    Classification: Unclassified
           Product: gcc
           Version: 4.7.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: adam@consulting.net.nz


I have encountered a weird Thread Local Storage issue where negative constants
in an array lookup statement are not combined into a single memory load
instruction. Non-negative constant are handled OK.

$ cat
tls_array_lookup_with_negative_constant_not_combined_into_a_single_instruction.c 
#include <stdint.h>

uint8_t array[64];
__thread uint8_t tls_array[64];

uint8_t array_lookup_with_positive_constant(int64_t position) {
  return array[position + 1];
}

uint8_t array_lookup_with_negative_constant(int64_t position) {
  return array[position - 1];
}

uint8_t tls_array_lookup_with_positive_constant(int64_t position) {
  return tls_array[position + 1];
}

uint8_t tls_array_lookup_with_negative_constant(int64_t position) {
  return tls_array[position - 1];
}

int main(void) {
  return 0;
}

$ gcc -O3 -std=gnu11 -pthread
tls_array_lookup_with_negative_constant_not_combined_into_a_single_instruction.c
&& objdump -d -m i386:x86-64:intel a.out |less

0000000000400540 <array_lookup_with_positive_constant>:
  400540:       0f b6 87 e1 09 60 00    movzx  eax,BYTE PTR [rdi+0x6009e1]
  400547:       c3                      ret    
  400548:       0f 1f 84 00 00 00 00    nop    DWORD PTR [rax+rax*1+0x0]
  40054f:       00 

0000000000400550 <array_lookup_with_negative_constant>:
  400550:       0f b6 87 df 09 60 00    movzx  eax,BYTE PTR [rdi+0x6009df]
  400557:       c3                      ret    
  400558:       0f 1f 84 00 00 00 00    nop    DWORD PTR [rax+rax*1+0x0]
  40055f:       00 

0000000000400560 <tls_array_lookup_with_positive_constant>:
  400560:       64 0f b6 87 c1 ff ff    movzx  eax,BYTE PTR fs:[rdi-0x3f]
  400567:       ff 
  400568:       c3                      ret    
  400569:       0f 1f 80 00 00 00 00    nop    DWORD PTR [rax+0x0]

0000000000400570 <tls_array_lookup_with_negative_constant>:
  400570:       48 c7 c0 c0 ff ff ff    mov    rax,0xffffffffffffffc0
  400577:       64 0f b6 44 07 ff       movzx  eax,BYTE PTR fs:[rdi+rax*1-0x1]
  40057d:       c3                      ret    
  40057e:       90                      nop
  40057f:       90                      nop

I believe tls_array_lookup_with_negative_constant should generate the
instruction movzx eax,BYTE PTR fs:[rdi-0x41]

Note: I initially observed the issue with the cmp instruction (e.g. mov
rax,0xfffffffffffffdc0; cmp BYTE PTR fs:[rax+rcx*1-0x80],r8b) so the issue is
likely to apply to all x86-64 instructions that perform a TLS memory load with
a negative constant.

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[adam at consulting dot net.nz]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

--- Comment #1 from Adam Warner <adam at consulting dot net.nz> 2012-09-01 03:00:39 UTC ---
Another example:
$ cat negative_constant_not_combined_into_a_single_instruction_example_2.c 
#include <stdint.h>

__thread uint8_t byte_array[100];

uint64_t lookup_with_positive_constant(int64_t offset1, int64_t offset2) {
  return *((uint64_t *) (byte_array + 8*offset1 + offset2 + 1));
}

uint64_t lookup_with_negative_constant(int64_t offset1, int64_t offset2) {
  return *((uint64_t *) (byte_array + 8*offset1 + offset2 - 1));
}

int main(void) {
  return 0;
}

$ gcc -O3 -std=gnu11 -pthread
negative_constant_not_combined_into_a_single_instruction_example_2.c && objdump
-d -m i386:x86-64:intel a.out |less

0000000000400540 <lookup_with_positive_constant>:
  400540:       64 48 8b 84 fe 9d ff    mov    rax,QWORD PTR
fs:[rsi+rdi*8-0x63]
  400547:       ff ff 
  400549:       c3                      ret    

0000000000400550 <lookup_with_negative_constant>:
  400550:       64 48 8b 14 25 00 00    mov    rdx,QWORD PTR fs:0x0
  400557:       00 00 
  400559:       48 8d 04 fe             lea    rax,[rsi+rdi*8]
  40055d:       48 81 c2 9c ff ff ff    add    rdx,0xffffffffffffff9c
  400564:       48 8b 44 02 ff          mov    rax,QWORD PTR [rdx+rax*1-0x1]
  400569:       c3                      ret    

The memory load expands into four instructions when subtracting 1 from the byte
offset instead of adding 1.

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[ubizjak at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

--- Comment #2 from Uros Bizjak <ubizjak at gmail dot com> 2012-09-01 14:53:44 UTC ---
This is due to the definition of x86_64_immediate_operand predicate, following
part:

        case UNSPEC:
          switch (XINT (op1, 1))
        {
        case UNSPEC_DTPOFF:
        case UNSPEC_NTPOFF:
          if (offset > 0
              && trunc_int_for_mode (offset, SImode) == offset)
            return true;
        }

I don't know why negative offsets are not allowed here.

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

--- Comment #3 from H.J. Lu <hjl.tools at gmail dot com> 2012-09-01 16:59:20 UTC ---
I am testing this patch:

diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index c78384b..ee7ae49 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -228,8 +228,7 @@
         {
         case UNSPEC_DTPOFF:
         case UNSPEC_NTPOFF:
-          if (offset > 0
-              && trunc_int_for_mode (offset, SImode) == offset)
+          if (trunc_int_for_mode (offset, SImode) == offset)
             return true;
         }
           break;

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at redhat dot com

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2012-09-02 14:06:26 UTC ---
(In reply to comment #2)
> This is due to the definition of x86_64_immediate_operand predicate, following
> part:
> 
>         case UNSPEC:
>           switch (XINT (op1, 1))
>         {
>         case UNSPEC_DTPOFF:
>         case UNSPEC_NTPOFF:
>           if (offset > 0
>               && trunc_int_for_mode (offset, SImode) == offset)
>             return true;
>         }
> 
> I don't know why negative offsets are not allowed here.

It was added by

http://gcc.gnu.org/ml/gcc-cvs/2002-10/msg00685.html
http://gcc.gnu.org/git/?p=gcc.git;a=commit;h=29d8dd5cee1f9ebd56b4473a690fcc54ad986265

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2012-09/msg00039.htm
                   |                            |l
   Last reconfirmed|                            |2012-09-02
   Target Milestone|---                         |4.8.0
     Ever Confirmed|0                           |1

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> 2012-09-02 14:21:41 UTC ---
A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00039.html

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[hjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

--- Comment #6 from hjl at gcc dot gnu.org <hjl at gcc dot gnu.org> 2012-09-12 18:09:08 UTC ---
Author: hjl
Date: Wed Sep 12 18:08:59 2012
New Revision: 191230

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191230
Log:
Allow negative offset for UNSPEC_DTPOFF/UNSPEC_NTPOFF

gcc/

    PR target/54445
    * config/i386/predicates.md (x86_64_immediate_operand): Allow
    negative offset for UNSPEC_DTPOFF/UNSPEC_NTPOFF.

gcc/testsuite/

    PR target/54445
    * gcc.target/i386/pr54445-1.c: New file.
    * gcc.target/i386/pr54445-2.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr54445-1.c
    trunk/gcc/testsuite/gcc.target/i386/pr54445-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/predicates.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #6 from hjl at gcc dot gnu.org <hjl at gcc dot gnu.org> 2012-09-12 18:09:08 UTC ---
Author: hjl
Date: Wed Sep 12 18:08:59 2012
New Revision: 191230

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=191230
Log:
Allow negative offset for UNSPEC_DTPOFF/UNSPEC_NTPOFF

gcc/

    PR target/54445
    * config/i386/predicates.md (x86_64_immediate_operand): Allow
    negative offset for UNSPEC_DTPOFF/UNSPEC_NTPOFF.

gcc/testsuite/

    PR target/54445
    * gcc.target/i386/pr54445-1.c: New file.
    * gcc.target/i386/pr54445-2.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr54445-1.c
    trunk/gcc/testsuite/gcc.target/i386/pr54445-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/predicates.md
    trunk/gcc/testsuite/ChangeLog

--- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> 2012-09-12 18:10:28 UTC ---
Fixed for 4.8.0.

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[hjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> 2012-09-12 18:10:28 UTC ---
Fixed for 4.8.0.

[Bug target/54445] TLS array lookup with negative constant is not combined into a single instruction

[adam at consulting dot net.nz]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54445

--- Comment #8 from Adam Warner <adam at consulting dot net.nz> 2012-09-13 09:01:44 UTC ---
Awesome! Thanks Uros Bizjak and H.J. Lu for locating and fixing the bug.