diff mbox

[Ada] Do not pass -Werror during linking

Message ID 201202072211.25618.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Feb. 7, 2012, 9:11 p.m. UTC
This breaks LTO bootstrap because of warnings for apparently incompatible types 
at the interface between C and Ada.  Given that it's very likely not possible 
to fix them all, let's keep accepting them.

Tested on i586-suse-linux, applied on the mainline.


2012-02-07  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc-interface/Make-lang.in (GCC_LINKERFLAGS): New variable.
	(GCC_LINK): Use it.

Comments

Richard Biener Feb. 8, 2012, 9:43 a.m. UTC | #1
On Tue, Feb 7, 2012 at 10:11 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> This breaks LTO bootstrap because of warnings for apparently incompatible types
> at the interface between C and Ada.  Given that it's very likely not possible
> to fix them all, let's keep accepting them.
>
> Tested on i586-suse-linux, applied on the mainline.

Can you try to extract a testcase (assuming it's just a single case?).
 We shouldn't
warn for layout-compatible types (but we may do so if for example struct
nesting differs).

Thanks,
Richard.

>
> 2012-02-07  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * gcc-interface/Make-lang.in (GCC_LINKERFLAGS): New variable.
>        (GCC_LINK): Use it.
>
>
> --
> Eric Botcazou
Eric Botcazou Feb. 10, 2012, 11:50 p.m. UTC | #2
> Can you try to extract a testcase (assuming it's just a single case?).
> We shouldn't
> warn for layout-compatible types (but we may do so if for example struct
> nesting differs).

It's more basic than that: for example, we map pointers on the C side to 
addresses (integers) on the Ada side.
Duncan Sands Feb. 11, 2012, 6:45 a.m. UTC | #3
Hi Eric,

>> Can you try to extract a testcase (assuming it's just a single case?).
>> We shouldn't
>> warn for layout-compatible types (but we may do so if for example struct
>> nesting differs).
>
> It's more basic than that: for example, we map pointers on the C side to
> addresses (integers) on the Ada side.

is having Address be an integer useful any more?  Nowadays it should be possible
to declare Address to be an access type with no associated storage pool.  Even
nicer might be to have it be turned into void* when lowering to GCC types.
After all, Address is really used much like void*; see how GNAT declares malloc
for example.  Both of these possibilities would probably play better with GCC's
middle end type system, which considers integers to be different to pointers.

Ciao, Duncan.

PS: I first thought of this when I noticed (when using the dragonegg plugin to
compile Ada) that some of LLVM's optimizations bail out when they see integers
being used where they expect a pointer.  I tried tweaking the declaration of
Address to be an access type as mentioned above, but unfortunately that crashes
the compiler pretty quickly.
Eric Botcazou Feb. 11, 2012, 10:37 a.m. UTC | #4
> is having Address be an integer useful any more?  Nowadays it should be
> possible to declare Address to be an access type with no associated storage
> pool.  Even nicer might be to have it be turned into void* when lowering to
> GCC types. After all, Address is really used much like void*; see how GNAT
> declares malloc for example.  Both of these possibilities would probably
> play better with GCC's middle end type system, which considers integers to
> be different to pointers.

The polymorphism pointer/address indeed proves to be problematic in certain 
circumstances (e.g. it breaks on m68k, see PR ada/48835).  My understanding is 
that using pointers in Ada is heavyweight, hence the choice of an integer for 
System.Address; and I'm reluctant to enter the type morphing business in gigi.

But we could investigate changing System.Memory to use an access type instead 
of System.Address.  It's a compiler unit so the impact should be limited, and 
at least this would solve the m68k problem.
Geert Bosch Feb. 11, 2012, 1:29 p.m. UTC | #5
On Feb 11, 2012, at 05:37, Eric Botcazou wrote:
> The polymorphism pointer/address indeed proves to be problematic in certain 
> circumstances (e.g. it breaks on m68k, see PR ada/48835).  My understanding is 
> that using pointers in Ada is heavyweight, hence the choice of an integer for 
> System.Address; and I'm reluctant to enter the type morphing business in gigi.

There are two issues:

  - In the Ada front end and run time, we rely on Address being an unsigned 
    type, so we can do address arithmetic. Ada doesn't have the notion of
    pointer arithmetic.

  - Access types (pointers) in Ada carry not only carry the address, but also
    the bounds in case of unconstrained types (as you well know)

I agree it doesn't fit well to do that, and seems pointless as one cannot
dereference Address anyway, so we have conversions where needed.

> But we could investigate changing System.Memory to use an access type instead 
> of System.Address.  It's a compiler unit so the impact should be limited, and 
> at least this would solve the m68k problem.

Probably for Convention C, we should convert Address to *void.

So, for

   function Alloc (Size : size_t) return System.Address;
   pragma Export (C, Alloc,   "__gnat_malloc");

we probably should do this replacement and add the necessary
type conversions. That would seem to be how this was intended.
However, I'm not sure how much of an earthquake this would
be in gigi.

  -Geert
Eric Botcazou Feb. 11, 2012, 2:40 p.m. UTC | #6
> Probably for Convention C, we should convert Address to *void.
>
> So, for
>
>    function Alloc (Size : size_t) return System.Address;
>    pragma Export (C, Alloc,   "__gnat_malloc");
>
> we probably should do this replacement and add the necessary
> type conversions. That would seem to be how this was intended.
> However, I'm not sure how much of an earthquake this would
> be in gigi.

That sounds like a good idea which, even if it doesn't solve the underlying 
issue in its full generality, would probably be sufficient in the m68k case.

I'll give it a try.  Thanks for the tip!
diff mbox

Patch

Index: gcc-interface/Make-lang.in
===================================================================
--- gcc-interface/Make-lang.in	(revision 183906)
+++ gcc-interface/Make-lang.in	(working copy)
@@ -165,7 +165,10 @@  else
   endif
 endif
 
-GCC_LINK=$(LINKER) $(ALL_LINKERFLAGS) -static-libgcc $(LDFLAGS)
+# Strip -Werror during linking for the LTO bootstrap
+GCC_LINKERFLAGS = $(filter-out -Werror, $(ALL_LINKERFLAGS))
+
+GCC_LINK=$(LINKER) $(GCC_LINKERFLAGS) -static-libgcc $(LDFLAGS)
 
 # Lists of files for various purposes.