public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/99470] New: Convert fixed index addition to array address offset
@ 2021-03-08 17:20 redbeard0531 at gmail dot com
  2021-03-08 18:05 ` [Bug rtl-optimization/99470] " pinskia at gcc dot gnu.org
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: redbeard0531 at gmail dot com @ 2021-03-08 17:20 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99470

            Bug ID: 99470
           Summary: Convert fixed index addition to array address offset
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redbeard0531 at gmail dot com
  Target Milestone: ---

These two functions do the same thing but f() is the cleaner source code
(especially when arr is a std::array) while g() generates better code:

https://gcc.godbolt.org/z/vTT399

#include <cstdint>

inline int8_t arr[256];

bool f(int a, int b) {
    return arr[a+128] == arr[b+128];
}

bool g(int a, int b) {
    return (arr+128)[a] == (arr+128)[b];
}

f(int, int):
        sub     edi, -128
        sub     esi, -128
        lea     rax, arr[rip]
        movsx   rdi, edi
        movsx   rsi, esi
        movzx   edx, BYTE PTR [rax+rsi]
        cmp     BYTE PTR [rax+rdi], dl
        sete    al
        ret
g(int, int):
        lea     rax, arr[rip+128]
        movsx   rdi, edi
        movsx   rsi, esi
        movzx   edx, BYTE PTR [rax+rsi]
        cmp     BYTE PTR [rax+rdi], dl
        sete    al
        ret

In addition to only doing the +128 once, it also ends up being completely free
in g() because the assembler (or linker?) folds the addition into the address
calculation by adjusting the offset of the rip-relative address. In the godbolt
link, you can see that when compiled to binary, the LEA instruction uses the
same form in both f() and g(), so the addition really is free in g().

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug rtl-optimization/99470] Convert fixed index addition to array address offset
  2021-03-08 17:20 [Bug c++/99470] New: Convert fixed index addition to array address offset redbeard0531 at gmail dot com
@ 2021-03-08 18:05 ` pinskia at gcc dot gnu.org
  2021-03-08 18:24 ` pinskia at gcc dot gnu.org
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-03-08 18:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99470

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
these two code are not equivalent at all due to overflows and such.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug rtl-optimization/99470] Convert fixed index addition to array address offset
  2021-03-08 17:20 [Bug c++/99470] New: Convert fixed index addition to array address offset redbeard0531 at gmail dot com
  2021-03-08 18:05 ` [Bug rtl-optimization/99470] " pinskia at gcc dot gnu.org
@ 2021-03-08 18:24 ` pinskia at gcc dot gnu.org
  2021-03-08 18:24 ` pinskia at gcc dot gnu.org
  2021-03-09 10:03 ` redbeard0531 at gmail dot com
  3 siblings, 0 replies; 5+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-03-08 18:24 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99470

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
These functions have the same code gen though:
inline signed char arr[256];

bool f(unsigned long a, unsigned long b) {
    return arr[a+128] == arr[b+128];
}

bool g(unsigned long a, unsigned long b) {
    return (arr+128)[a] == (arr+128)[b];
}

----- CUT -----
The reason why int is not equivalent because signed integer overflow is
undefined plus doing the math in 64bit or 32bit would cause a huge difference
in some cases.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug rtl-optimization/99470] Convert fixed index addition to array address offset
  2021-03-08 17:20 [Bug c++/99470] New: Convert fixed index addition to array address offset redbeard0531 at gmail dot com
  2021-03-08 18:05 ` [Bug rtl-optimization/99470] " pinskia at gcc dot gnu.org
  2021-03-08 18:24 ` pinskia at gcc dot gnu.org
@ 2021-03-08 18:24 ` pinskia at gcc dot gnu.org
  2021-03-09 10:03 ` redbeard0531 at gmail dot com
  3 siblings, 0 replies; 5+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-03-08 18:24 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99470

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> The reason why int is not equivalent because signed integer overflow is
> undefined plus doing the math in 64bit or 32bit would cause a huge
> difference in some cases.
The cases is if a/b are negative.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug rtl-optimization/99470] Convert fixed index addition to array address offset
  2021-03-08 17:20 [Bug c++/99470] New: Convert fixed index addition to array address offset redbeard0531 at gmail dot com
                   ` (2 preceding siblings ...)
  2021-03-08 18:24 ` pinskia at gcc dot gnu.org
@ 2021-03-09 10:03 ` redbeard0531 at gmail dot com
  3 siblings, 0 replies; 5+ messages in thread
From: redbeard0531 at gmail dot com @ 2021-03-09 10:03 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99470

Mathias Stearn <redbeard0531 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |---

--- Comment #4 from Mathias Stearn <redbeard0531 at gmail dot com> ---
Yes, but I believe any case where they would access different addresses would
be UB overflow in f(), making it valid to turn f() into g(), especially if you
used an internal lowering which sign extended index to pointer width and had
defined wrapping semantics. I'll note that clang already generates identical
code for f() and g() https://gcc.godbolt.org/z/j897sh, although I think gcc has
better codegen at least for g().

Also, my example was perhaps oversimplified. My indexes were actually int8_t
(which is why I'm indexing into a 256-element array), so due to int promotion,
overflow is actually impossible. However, with int8_t arguments, gcc generates
even worse code for f(), doing the sign-extension twice for some reason (8 ->
32 -> 64): https://gcc.godbolt.org/z/5r9h89


(I hope it isn't a faux pas to reopen the ticket, but I think I've provided
enough new information that this warrants another look)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-03-09 10:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 17:20 [Bug c++/99470] New: Convert fixed index addition to array address offset redbeard0531 at gmail dot com
2021-03-08 18:05 ` [Bug rtl-optimization/99470] " pinskia at gcc dot gnu.org
2021-03-08 18:24 ` pinskia at gcc dot gnu.org
2021-03-08 18:24 ` pinskia at gcc dot gnu.org
2021-03-09 10:03 ` redbeard0531 at gmail dot com

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).