[Bug ada/62117] New: [4.9 regression] wrong code for passing small array argument'Address, in generic

[Bug ada/62117] New: [4.9 regression] wrong code for passing small array argument'Address, in generic

[demoonlit at panathenaia dot halfmoon.jp]

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

            Bug ID: 62117
           Summary: [4.9 regression] wrong code for passing small array
                    argument'Address, in generic
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ada
          Assignee: unassigned at gcc dot gnu.org
          Reporter: demoonlit at panathenaia dot halfmoon.jp

Created attachment 33306
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33306&action=edit
minimal bug triggering source code

When instantiating a generic function like below, with a small array type
placed in register, according to x86_64 System V calling convension,

   generic
      type Key_Type is limited private;
   function Compare (Left, Right : Key_Type) return Integer;

'Address attributes of arguments point wrong place.
The compiler probably has forgotten to store the arguments to memory.

For example, if the body calls memcmp,

   function Compare (Left, Right : Key_Type) return Integer is
   begin
      return memcmp (Left'Address, Right'Address, Key_Type'Size / 8);
   end Compare;

The generated code compiled with -S -Og:

==== gcc-4.9 ====
_inst__compare:
LFB1:
    subq    $40, %rsp
LCFI0:
    movq    %rsp, %rsi  # Left and Right are passed by %rdi and %rsi
    leaq    16(%rsp), %rdi # but this generated code overwrites them
    movl    $4, %edx
    call    _memcmp
    addq    $40, %rsp
LCFI1:
    ret

This bug has appeared from gcc-4.9. gcc-4.8 is ok.

=== gcc-4.8 ====
_inst__compare:
LFB1:
    subq    $40, %rsp
LCFI0:
    movl    %edi, 16(%rsp) # Left and Right are stored on the stack
    movl    %esi, (%rsp)
    movq    %rsp, %rsi
    leaq    16(%rsp), %rdi
    movl    $4, %edx
    call    _memcmp
    addq    $40, %rsp
LCFI1:
    ret

[Bug ada/62117] [4.9 regression] wrong code for passing small array argument'Address, in generic

[demoonlit at panathenaia dot halfmoon.jp]

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

yuta tomino <demoonlit at panathenaia dot halfmoon.jp> changed:

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

--- Comment #2 from yuta tomino <demoonlit at panathenaia dot halfmoon.jp> ---
Thank you for looking, Eric.

Surely, as you say, memcmp is unsuitable for presenting an example.
It's my mistake. But I had not found other case at the time.

I found another case since then. Pure is not a trigger in this case.
Please look this, too.

In the new case, it uses Float'Valid attribute calling
System.Fat_Flt.Unaligned_Valid implicitly.
Unaligned_Valid takes an Address argument. But it's not marked as pure.
And, it's a detail of compiler implementation. I think this behavior should not
affect application code.

It needs the type U for calling Unaligned_Valid.
If this type is removed (and some other method like Unchecked_Conversion is
used), System.Fat_Flt.Valid may be called instead of Unaligned_Valid.
System.Fat_Flt.Valid works correctly.

I apologize for taking your time.

-- case2.ads
package case2 is
   --  not pure package

   function Packed_Unaligned_Valid (Item : Long_Long_Integer) return Boolean;
   --  not pure function

end case2;

-- case2.adb
with system.storage_elements;
package body case2 is
   use type System.Storage_Elements.Storage_Offset;

   --  A Float value is packed into the argument Item, at unaligned position.
   type U is record
      C : Character;
      F : Float;
   end record;
   pragma Pack (U);

   function Packed_Unaligned_Valid (Item : Long_Long_Integer) return Boolean is
      X : U;
      for X'Address use Item'Address;
   begin
      return X.F'Valid; -- implicit calling System.Fat_Flt.Unaligned_Valid
   end Packed_Unaligned_Valid;

end case2;

==== gcc-4.9 ====
_case2__packed_unaligned_valid:
LFB3:
    subq    $24, %rsp
LCFI0:
    leaq    9(%rsp), %rdi
    call    _system__fat_flt__attr_float__unaligned_valid
    addq    $24, %rsp
LCFI1:
    ret

=== gcc-4.8 ====
_case2__packed_unaligned_valid:
LFB3:
    subq    $24, %rsp
LCFI0:
    movq    %rdi, 8(%rsp) # this operation is missing in gcc-4.9
    leaq    9(%rsp), %rdi
    call    _system__fat_flt__attr_float__unaligned_valid
    addq    $24, %rsp
LCFI1:
    ret

[Bug ada/62117] [4.9 regression] wrong code for passing small array argument'Address, in generic

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.9.3

[Bug ada/62117] [4.9 regression] wrong code for passing small array argument'Address, in generic

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4

[Bug ada/62117] [4.9 regression] wrong code for passing small array argument'Address, in generic

[demoonlit at panathenaia dot halfmoon.jp]

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

--- Comment #3 from yuta tomino <demoonlit at panathenaia dot halfmoon.jp> ---
I understood by trial and error that pragma Pure_Function is equivalent to
__attribute__((const)).
I'm sorry. The first case (in my first description) is not compiler's bug but
my mistake. The parameters Left and Right were erased by optimization because
compiler assumed that Pure_Function does not dereference, in my current
understanding.

Also, the case2 (in comment 2) is not reproduced by gcc-5.
However, the reason is that System.Fat_Flt.Unaligned_Valid is removed on gcc-5.


...By the way, I found an another point that I'm concerned.

In the document of pragma Pure_Function, a function having Address parameter is
not considered as Pure_Function.

https://gcc.gnu.org/onlinedocs/gnat_rm/Pragma-Pure_005fFunction.html

> One exception is any function that has at least one formal of type System.Address or a type derived from it. Such functions are not considered pure by default,

But,

with System;
package case3_p is
   pragma Pure;
   function F (X : System.Address) return System.Address;
end case3_p;

package body case3_p is
   function F (X : System.Address) return System.Address is
   begin
      return X;
   end F;
end case3_p;

with case3_p;
with System.Storage_Elements; use System.Storage_Elements;
with GNAT.IO;
procedure case3_m is
begin
   GNAT.IO.Put (Integer (To_Integer (case3_p.F (System'To_Address (10)))));
   GNAT.IO.Put (Integer (To_Integer (case3_p.F (System'To_Address (10)))));
   GNAT.IO.Put (Integer (To_Integer (case3_p.F (System'To_Address (10)))));
end case3_m;

Compile this case3_m.adb with optimization.

% gcc -S -O -gnatp case3_m.adb

    .text
    .globl __ada_case3_m
__ada_case3_m:
LFB1:
    pushq    %rbx
LCFI0:
    movl    $10, %edi
    call    _case3_p__f
    movq    %rax, %rbx
    movl    %eax, %edi
    call    _gnat__io__put__2
    movl    %ebx, %edi
    call    _gnat__io__put__2
    movl    %ebx, %edi
    call    _gnat__io__put__2
    popq    %rbx
LCFI1:
    ret

_case3_p__f is called once on the machine code against that case3_p.F is called
three times from case3_m.
Perhaps, case3_p.F is treated as Pure_Function.

Which of those is correct, the document or the behaviour of compiler?

[Bug ada/62117] function taking System.Address wrongly considered pure

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-02-02
            Summary|[4.9 regression] wrong code |function taking
                   |for passing small array     |System.Address wrongly
                   |argument'Address, in        |considered pure
                   |generic                     |
     Ever confirmed|0                           |1

--- Comment #4 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Which of those is correct, the document or the behaviour of compiler?

The documentation, this looks like an oversight in the implementation.

[Bug ada/62117] function taking System.Address wrongly considered as Pure

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.9.3 has been released.

[Bug ada/62117] function taking System.Address wrongly considered as Pure

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.9.3                       |4.9.4