diff mbox

ptx preliminary address space fixes [1/4]

Message ID 54117552.2040200@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt Sept. 11, 2014, 10:11 a.m. UTC
I'm getting ready to submit the ptx port and the various changes that 
are necessary for it. To start with, here are some patches to deal with 
address space issues.

ptx has the concept of an implicit address space: everything (objects on 
the stack as well as constant data and global variables) lives in its 
own address space. These are applied by a lower-as pass which I'll 
submit later.

Since address spaces are more pervasive than on other targets and can 
show up in places the compiler doesn't yet expect them (such as local 
variables), this uncovers a few bugs in the optimizers. These are 
typically of the kind where we recreate a memory reference and aren't 
quite careful enough to preserve the existing address space.

The first patch below just introduces a utility function that the 
following patches will use.  All were bootstrapped and tested together 
on x86_64-linux. Ok?


Bernd

Comments

Richard Biener Sept. 11, 2014, 11:29 a.m. UTC | #1
On Thu, Sep 11, 2014 at 12:11 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> I'm getting ready to submit the ptx port and the various changes that are
> necessary for it. To start with, here are some patches to deal with address
> space issues.
>
> ptx has the concept of an implicit address space: everything (objects on the
> stack as well as constant data and global variables) lives in its own
> address space. These are applied by a lower-as pass which I'll submit later.
>
> Since address spaces are more pervasive than on other targets and can show
> up in places the compiler doesn't yet expect them (such as local variables),
> this uncovers a few bugs in the optimizers. These are typically of the kind
> where we recreate a memory reference and aren't quite careful enough to
> preserve the existing address space.
>
> The first patch below just introduces a utility function that the following
> patches will use.  All were bootstrapped and tested together on
> x86_64-linux. Ok?

+tree
+apply_as_to_type (tree type, addr_space_t as)
+{
+  int quals = TYPE_QUALS_NO_ADDR_SPACE (type);
+  if (!ADDR_SPACE_GENERIC_P (as))
+    quals |= ENCODE_QUAL_ADDR_SPACE (as);
+  type = build_qualified_type (type, quals);

please optimize the case of quals == TYPE_QUALS (type).

+  if (TREE_CODE (type) == ARRAY_TYPE)
+    TREE_TYPE (type) = apply_as_to_type (TREE_TYPE (type), as);

why is this necessary for ARRAY_TYPE but not for sth like
a RECORD_TYPE or a POINTER_TYPE?

[having address-spaces on types rather than decls and (indirect)
memory references seems odd anyway - that is, as far as the
middle-/back-end
is concerned]

The name apply_as_to_type looks odd to me - other address-space
related functions use addr_space - can you change it to that please?

Thanks,
Richard.

>
> Bernd
Bernd Schmidt Sept. 12, 2014, 11:15 a.m. UTC | #2
On 09/11/2014 01:29 PM, Richard Biener wrote:
> +  if (TREE_CODE (type) == ARRAY_TYPE)
> +    TREE_TYPE (type) = apply_as_to_type (TREE_TYPE (type), as);
>
> why is this necessary for ARRAY_TYPE but not for sth like
> a RECORD_TYPE or a POINTER_TYPE?

Still testing whether I actually strictly need it for ARRAY_TYPE 
nowadays (these patches are really old...). However, the TYPE_FIELDS of 
a RECORD_TYPE seem to be mostly ignored once the frontends are done, but 
it's very easy for other parts of the compiler to take the TREE_TYPE of 
an ARRAY_TYPE. Fixing that up is simple and seems like a good thing to 
do for consistency (I notice that maybe I should add VECTOR_TYPE).

For a POINTER_TYPE, it is correct not to modify the pointed-to type. We 
want to express that a variable of that pointer type lives in an address 
space, not that the pointed-to type is different.

> The name apply_as_to_type looks odd to me - other address-space
> related functions use addr_space - can you change it to that please?

Will change, and update the other patches accordingly.


Bernd
Richard Biener Sept. 12, 2014, 11:48 a.m. UTC | #3
On Fri, Sep 12, 2014 at 1:15 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 09/11/2014 01:29 PM, Richard Biener wrote:
>>
>> +  if (TREE_CODE (type) == ARRAY_TYPE)
>> +    TREE_TYPE (type) = apply_as_to_type (TREE_TYPE (type), as);
>>
>> why is this necessary for ARRAY_TYPE but not for sth like
>> a RECORD_TYPE or a POINTER_TYPE?
>
>
> Still testing whether I actually strictly need it for ARRAY_TYPE nowadays
> (these patches are really old...). However, the TYPE_FIELDS of a RECORD_TYPE
> seem to be mostly ignored once the frontends are done, but it's very easy
> for other parts of the compiler to take the TREE_TYPE of an ARRAY_TYPE.
> Fixing that up is simple and seems like a good thing to do for consistency
> (I notice that maybe I should add VECTOR_TYPE).

Well, for an access a->b the COMPONENT_REF specifies the type
of the reference which uses the type of the FIELD_DECL...  IVOPTs
for example may produce

 ptr *p = &a->b;
 *p;

from that with ptr * built from TREE_TYPE of that expression.

Btw, a similar type as VECTOR_TYPE is COMPLEX_TYPE.

> For a POINTER_TYPE, it is correct not to modify the pointed-to type. We want
> to express that a variable of that pointer type lives in an address space,
> not that the pointed-to type is different.
>
>> The name apply_as_to_type looks odd to me - other address-space
>> related functions use addr_space - can you change it to that please?
>
>
> Will change, and update the other patches accordingly.

Thanks,
Richard.

>
> Bernd
>
Bernd Schmidt Sept. 12, 2014, 11:56 a.m. UTC | #4
On 09/12/2014 01:48 PM, Richard Biener wrote:
>> Still testing whether I actually strictly need it for ARRAY_TYPE nowadays
>> (these patches are really old...). However, the TYPE_FIELDS of a RECORD_TYPE
>> seem to be mostly ignored once the frontends are done, but it's very easy
>> for other parts of the compiler to take the TREE_TYPE of an ARRAY_TYPE.
>> Fixing that up is simple and seems like a good thing to do for consistency
>> (I notice that maybe I should add VECTOR_TYPE).
>
> Well, for an access a->b the COMPONENT_REF specifies the type
> of the reference which uses the type of the FIELD_DECL...  IVOPTs
> for example may produce
>
>   ptr *p = &a->b;
>   *p;
>
> from that with ptr * built from TREE_TYPE of that expression.

Yes, but that expression is the COMPONENT_REF. While that may initially 
use the type from the FIELD_DECL, afterwards it is independent from it 
(and types on COMPONENT_REFs and ARRAY_REFs are changed by the lower-as 
pass on ptx).  We're not really looking at the FIELD_DECLs for anything 
important AFAIK.


Bernd
Richard Biener Sept. 12, 2014, 12:37 p.m. UTC | #5
On Fri, Sep 12, 2014 at 1:56 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 09/12/2014 01:48 PM, Richard Biener wrote:
>>>
>>> Still testing whether I actually strictly need it for ARRAY_TYPE nowadays
>>> (these patches are really old...). However, the TYPE_FIELDS of a
>>> RECORD_TYPE
>>> seem to be mostly ignored once the frontends are done, but it's very easy
>>> for other parts of the compiler to take the TREE_TYPE of an ARRAY_TYPE.
>>> Fixing that up is simple and seems like a good thing to do for
>>> consistency
>>> (I notice that maybe I should add VECTOR_TYPE).
>>
>>
>> Well, for an access a->b the COMPONENT_REF specifies the type
>> of the reference which uses the type of the FIELD_DECL...  IVOPTs
>> for example may produce
>>
>>   ptr *p = &a->b;
>>   *p;
>>
>> from that with ptr * built from TREE_TYPE of that expression.
>
>
> Yes, but that expression is the COMPONENT_REF. While that may initially use
> the type from the FIELD_DECL, afterwards it is independent from it (and
> types on COMPONENT_REFs and ARRAY_REFs are changed by the lower-as pass on
> ptx).  We're not really looking at the FIELD_DECLs for anything important
> AFAIK.

You figured out SRA yourself.  Btw, I still detest the use of a lowering
pass for PTX (changing types in-place even more so).  Maybe you
want to do the lowering on RTL where you can simply adjust the
affected MEMs MEM_ATTRs.  And wouldn't it be nice if you can
do similar things on GIMPLE? ;)

Richard.

>
> Bernd
>
Bernd Schmidt Sept. 16, 2014, 11:24 a.m. UTC | #6
On 09/12/2014 01:48 PM, Richard Biener wrote:
> On Fri, Sep 12, 2014 at 1:15 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> On 09/11/2014 01:29 PM, Richard Biener wrote:
>>>
>>> +  if (TREE_CODE (type) == ARRAY_TYPE)
>>> +    TREE_TYPE (type) = apply_as_to_type (TREE_TYPE (type), as);
>>>
>>> why is this necessary for ARRAY_TYPE but not for sth like
>>> a RECORD_TYPE or a POINTER_TYPE?
>>
>>
>> Still testing whether I actually strictly need it for ARRAY_TYPE nowadays
>> (these patches are really old...). However, the TYPE_FIELDS of a RECORD_TYPE
>> seem to be mostly ignored once the frontends are done, but it's very easy
>> for other parts of the compiler to take the TREE_TYPE of an ARRAY_TYPE.
>> Fixing that up is simple and seems like a good thing to do for consistency
>> (I notice that maybe I should add VECTOR_TYPE).
>
> Well, for an access a->b the COMPONENT_REF specifies the type
> of the reference which uses the type of the FIELD_DECL...  IVOPTs
> for example may produce
>
>   ptr *p = &a->b;
>   *p;
>
> from that with ptr * built from TREE_TYPE of that expression.
>
> Btw, a similar type as VECTOR_TYPE is COMPLEX_TYPE.

Ok, so testing seems to show that nothing breaks with the ARRAY_TYPE 
special case removed. However, I remembered another reason to do this, 
and it's for consistency with how address spaces are represented in 
other parts of the compiler - specifically, the C frontend.

C has the notion that arrays don't have type qualifiers, so to get the 
address space of an array you'd have to look at the address space of its 
element types. Joseph has in the past rejected patches to fix this 
inconsistency. For other types like structs or vectors (as we saw in the 
tree-vect patch) it's the outermost type that has the address space 
information.

I guess I'll declare myself agnostic, let me know whatever variant you 
want to have here (fixing up all types or not fixing arrays) and I'll 
make a new patch.


Bernd
Richard Biener Sept. 16, 2014, 12:59 p.m. UTC | #7
On Tue, Sep 16, 2014 at 1:24 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 09/12/2014 01:48 PM, Richard Biener wrote:
>>
>> On Fri, Sep 12, 2014 at 1:15 PM, Bernd Schmidt <bernds@codesourcery.com>
>> wrote:
>>>
>>> On 09/11/2014 01:29 PM, Richard Biener wrote:
>>>>
>>>>
>>>> +  if (TREE_CODE (type) == ARRAY_TYPE)
>>>> +    TREE_TYPE (type) = apply_as_to_type (TREE_TYPE (type), as);
>>>>
>>>> why is this necessary for ARRAY_TYPE but not for sth like
>>>> a RECORD_TYPE or a POINTER_TYPE?
>>>
>>>
>>>
>>> Still testing whether I actually strictly need it for ARRAY_TYPE nowadays
>>> (these patches are really old...). However, the TYPE_FIELDS of a
>>> RECORD_TYPE
>>> seem to be mostly ignored once the frontends are done, but it's very easy
>>> for other parts of the compiler to take the TREE_TYPE of an ARRAY_TYPE.
>>> Fixing that up is simple and seems like a good thing to do for
>>> consistency
>>> (I notice that maybe I should add VECTOR_TYPE).
>>
>>
>> Well, for an access a->b the COMPONENT_REF specifies the type
>> of the reference which uses the type of the FIELD_DECL...  IVOPTs
>> for example may produce
>>
>>   ptr *p = &a->b;
>>   *p;
>>
>> from that with ptr * built from TREE_TYPE of that expression.
>>
>> Btw, a similar type as VECTOR_TYPE is COMPLEX_TYPE.
>
>
> Ok, so testing seems to show that nothing breaks with the ARRAY_TYPE special
> case removed. However, I remembered another reason to do this, and it's for
> consistency with how address spaces are represented in other parts of the
> compiler - specifically, the C frontend.
>
> C has the notion that arrays don't have type qualifiers, so to get the
> address space of an array you'd have to look at the address space of its
> element types. Joseph has in the past rejected patches to fix this
> inconsistency. For other types like structs or vectors (as we saw in the
> tree-vect patch) it's the outermost type that has the address space
> information.
>
> I guess I'll declare myself agnostic, let me know whatever variant you want
> to have here (fixing up all types or not fixing arrays) and I'll make a new
> patch.

Hmm.  How is it with other compositive types like vectors and complex?
It's bad that the middle-end needs to follow a specific frontends need.
Why's the representation tied so closely together?

OTOH that address-spaces are "qualifiers" is an implementation detail
(and maybe not the very best).  So I don't see how the C frontend
needs to view them as qualifiers?

Joseph?

Thanks,
Richard.

>
> Bernd
>
>
Joseph Myers Sept. 16, 2014, 8:56 p.m. UTC | #8
On Tue, 16 Sep 2014, Richard Biener wrote:

> Hmm.  How is it with other compositive types like vectors and complex?
> It's bad that the middle-end needs to follow a specific frontends need.
> Why's the representation tied so closely together?

Complex types aren't derived types in C terms; they don't have an element 
type, but a corresponding real type.  Vectors should presumably be treated 
like complex types.  So both can have qualifiers.

> OTOH that address-spaces are "qualifiers" is an implementation detail
> (and maybe not the very best).  So I don't see how the C frontend
> needs to view them as qualifiers?

It's not an implementation detail, it's how TR 18037 defines them, and 
thus how the C front end should represent them in order to follow the 
requirements of TR 18037.

If something different is appropriate on GIMPLE, when GIMPLE gets its own 
type system independent of trees then the lowering could of course change 
this sort of thing.

(I think the fixed-point support, also from TR 18037, would better be 
implemented through lowering from fixed-point types at front-end level to 
special (e.g. saturating) operations on normal types and modes, rather 
than carrying a load of special types and modes through to the back end.)
Bernd Schmidt Sept. 16, 2014, 9:06 p.m. UTC | #9
On 09/16/2014 10:56 PM, Joseph S. Myers wrote:
> On Tue, 16 Sep 2014, Richard Biener wrote:
>
>> Hmm.  How is it with other compositive types like vectors and complex?
>> It's bad that the middle-end needs to follow a specific frontends need.
>> Why's the representation tied so closely together?
>
> Complex types aren't derived types in C terms; they don't have an element
> type, but a corresponding real type.  Vectors should presumably be treated
> like complex types.  So both can have qualifiers.
>
>> OTOH that address-spaces are "qualifiers" is an implementation detail
>> (and maybe not the very best).  So I don't see how the C frontend
>> needs to view them as qualifiers?
>
> It's not an implementation detail, it's how TR 18037 defines them, and
> thus how the C front end should represent them in order to follow the
> requirements of TR 18037.

My position is that standards do not mandate how our internal data 
structures should look like, and we should be striving to make them 
consistent. That means building array types in such a way that address 
spaces (and probably things like constness) are identical between the 
array type and its element type. It would be easy enough to make a 
c_type_quals wrapper function around TYPE_QUALS that returns zero for 
array types, but I would not expect there to really be a need for it.


Bernd
Joseph Myers Sept. 16, 2014, 9:18 p.m. UTC | #10
On Tue, 16 Sep 2014, Bernd Schmidt wrote:

> > It's not an implementation detail, it's how TR 18037 defines them, and
> > thus how the C front end should represent them in order to follow the
> > requirements of TR 18037.
> 
> My position is that standards do not mandate how our internal data structures
> should look like, and we should be striving to make them consistent. That

My position is that the structures in the front end should correspond to 
how the language is actually defined, so that the most obvious way of 
accessing some property of an entity in the front end actually gets that 
property as it is defined in the standard, and not something similar but 
confusingly different defined by GCC.  It's the job of genericizing / 
gimplifying to convert from structures that closely correspond to the 
source program and the language standard into ones that are more 
convenient for language-independent processing and code generation.

(That TYPE_MAIN_VARIANT maps an array of qualified type to an array of 
corresponding unqualified type necessitates lots of special cases in the 
front end to avoid applying TYPE_MAIN_VARIANT to array types, since in C 
terms array types are always unqualified and are unrelated to an array of 
corresponding unqualified element type.)
Bernd Schmidt Sept. 16, 2014, 10:03 p.m. UTC | #11
On 09/16/2014 11:18 PM, Joseph S. Myers wrote:

> (That TYPE_MAIN_VARIANT maps an array of qualified type to an array of
> corresponding unqualified type necessitates lots of special cases in the
> front end to avoid applying TYPE_MAIN_VARIANT to array types, since in C
> terms array types are always unqualified and are unrelated to an array of
> corresponding unqualified element type.)

Sounds like you want a c_type_main_variant wrapper then? What exactly 
breaks if you ignore the problem and apply TYPE_MAIN_VARIANT to arrays?


bernd
Joseph Myers Sept. 16, 2014, 10:16 p.m. UTC | #12
On Wed, 17 Sep 2014, Bernd Schmidt wrote:

> On 09/16/2014 11:18 PM, Joseph S. Myers wrote:
> 
> > (That TYPE_MAIN_VARIANT maps an array of qualified type to an array of
> > corresponding unqualified type necessitates lots of special cases in the
> > front end to avoid applying TYPE_MAIN_VARIANT to array types, since in C
> > terms array types are always unqualified and are unrelated to an array of
> > corresponding unqualified element type.)
> 
> Sounds like you want a c_type_main_variant wrapper then? What exactly breaks
> if you ignore the problem and apply TYPE_MAIN_VARIANT to arrays?

Anything where the C standard defines something in terms of the 
unqualified versions of types, or the set of qualifiers on a type, 
operates incorrectly (tests compatibility of the wrong types, etc.) if you 
apply TYPE_MAIN_VARIANT to arrays.
Bernd Schmidt Sept. 26, 2014, noon UTC | #13
On 09/16/2014 02:59 PM, Richard Biener wrote:
> On Tue, Sep 16, 2014 at 1:24 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> Ok, so testing seems to show that nothing breaks with the ARRAY_TYPE special
>> case removed. However, I remembered another reason to do this, and it's for
>> consistency with how address spaces are represented in other parts of the
>> compiler - specifically, the C frontend.
>>
>> C has the notion that arrays don't have type qualifiers, so to get the
>> address space of an array you'd have to look at the address space of its
>> element types. Joseph has in the past rejected patches to fix this
>> inconsistency. For other types like structs or vectors (as we saw in the
>> tree-vect patch) it's the outermost type that has the address space
>> information.
>>
>> I guess I'll declare myself agnostic, let me know whatever variant you want
>> to have here (fixing up all types or not fixing arrays) and I'll make a new
>> patch.
>
> Hmm.  How is it with other compositive types like vectors and complex?
> It's bad that the middle-end needs to follow a specific frontends need.
> Why's the representation tied so closely together?
>
> OTOH that address-spaces are "qualifiers" is an implementation detail
> (and maybe not the very best).  So I don't see how the C frontend
> needs to view them as qualifiers?

So what's the conclusion here? What should I be doing with the patch?


Bernd
Richard Biener Sept. 26, 2014, 12:05 p.m. UTC | #14
On Fri, Sep 26, 2014 at 2:00 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 09/16/2014 02:59 PM, Richard Biener wrote:
>>
>> On Tue, Sep 16, 2014 at 1:24 PM, Bernd Schmidt <bernds@codesourcery.com>
>> wrote:
>>>
>>> Ok, so testing seems to show that nothing breaks with the ARRAY_TYPE
>>> special
>>> case removed. However, I remembered another reason to do this, and it's
>>> for
>>> consistency with how address spaces are represented in other parts of the
>>> compiler - specifically, the C frontend.
>>>
>>> C has the notion that arrays don't have type qualifiers, so to get the
>>> address space of an array you'd have to look at the address space of its
>>> element types. Joseph has in the past rejected patches to fix this
>>> inconsistency. For other types like structs or vectors (as we saw in the
>>> tree-vect patch) it's the outermost type that has the address space
>>> information.
>>>
>>> I guess I'll declare myself agnostic, let me know whatever variant you
>>> want
>>> to have here (fixing up all types or not fixing arrays) and I'll make a
>>> new
>>> patch.
>>
>>
>> Hmm.  How is it with other compositive types like vectors and complex?
>> It's bad that the middle-end needs to follow a specific frontends need.
>> Why's the representation tied so closely together?
>>
>> OTOH that address-spaces are "qualifiers" is an implementation detail
>> (and maybe not the very best).  So I don't see how the C frontend
>> needs to view them as qualifiers?
>
>
> So what's the conclusion here? What should I be doing with the patch?

If currently address-space support matches up with the C frontend
and the C standard then the middle-end has to cope with that.
In this case, cope with array element types not having address-space
qualifiers.

Richard.

>
> Bernd
>
Bernd Schmidt Sept. 26, 2014, 12:14 p.m. UTC | #15
On 09/26/2014 02:05 PM, Richard Biener wrote:
> If currently address-space support matches up with the C frontend
> and the C standard then the middle-end has to cope with that.
> In this case, cope with array element types not having address-space
> qualifiers.

That's the opposite of what happens. The C frontend makes array element 
types have address-space qualifiers but not the array type.


Bernd
Richard Biener Sept. 26, 2014, 12:26 p.m. UTC | #16
On Fri, Sep 26, 2014 at 2:14 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 09/26/2014 02:05 PM, Richard Biener wrote:
>>
>> If currently address-space support matches up with the C frontend
>> and the C standard then the middle-end has to cope with that.
>> In this case, cope with array element types not having address-space
>> qualifiers.
>
>
> That's the opposite of what happens. The C frontend makes array element
> types have address-space qualifiers but not the array type.

Ah, ok.  Then the opposite way around ;)

Richard.

>
> Bernd
>
>
Bernd Schmidt Sept. 26, 2014, 12:28 p.m. UTC | #17
On 09/26/2014 02:26 PM, Richard Biener wrote:
> On Fri, Sep 26, 2014 at 2:14 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> On 09/26/2014 02:05 PM, Richard Biener wrote:
>>>
>>> If currently address-space support matches up with the C frontend
>>> and the C standard then the middle-end has to cope with that.
>>> In this case, cope with array element types not having address-space
>>> qualifiers.
>>
>>
>> That's the opposite of what happens. The C frontend makes array element
>> types have address-space qualifiers but not the array type.
>
> Ah, ok.  Then the opposite way around ;)

Ok, so that means that my original patch which updated the element types 
for arrays is in fact the way to go?


Bernd
Richard Biener Sept. 26, 2014, 12:42 p.m. UTC | #18
On Fri, Sep 26, 2014 at 2:28 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 09/26/2014 02:26 PM, Richard Biener wrote:
>>
>> On Fri, Sep 26, 2014 at 2:14 PM, Bernd Schmidt <bernds@codesourcery.com>
>> wrote:
>>>
>>> On 09/26/2014 02:05 PM, Richard Biener wrote:
>>>>
>>>>
>>>> If currently address-space support matches up with the C frontend
>>>> and the C standard then the middle-end has to cope with that.
>>>> In this case, cope with array element types not having address-space
>>>> qualifiers.
>>>
>>>
>>>
>>> That's the opposite of what happens. The C frontend makes array element
>>> types have address-space qualifiers but not the array type.
>>
>>
>> Ah, ok.  Then the opposite way around ;)
>
>
> Ok, so that means that my original patch which updated the element types for
> arrays is in fact the way to go?

It seems to do both, apply the as to the array _and_ the element type, no?

Thus for arrays you'd need to do (in that old patches terms)

  type = build_variant_type_copy (type);
  TREE_TYPE (type) = apply_as_to_type (TREE_TYPE (type), as);

and drop the build_qualified_type call for the array type itself.  Oh,
and instead of unconditionally doing that copy walk the existing
variant list to see if there is aready a properly qualified variant.

(it seems to me the apply_as_to_type function should first check
if 'type' already has the appropriate address-space qualification).

Richard.

>
> Bernd
>
>
Bernd Schmidt Sept. 26, 2014, 3:14 p.m. UTC | #19
On 09/26/2014 02:42 PM, Richard Biener wrote:
> On Fri, Sep 26, 2014 at 2:28 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> On 09/26/2014 02:26 PM, Richard Biener wrote:
>>>
>>> On Fri, Sep 26, 2014 at 2:14 PM, Bernd Schmidt <bernds@codesourcery.com>
>>> wrote:
>>>>
>>>> On 09/26/2014 02:05 PM, Richard Biener wrote:
>>>>>
>>>>>
>>>>> If currently address-space support matches up with the C frontend
>>>>> and the C standard then the middle-end has to cope with that.
>>>>> In this case, cope with array element types not having address-space
>>>>> qualifiers.
>>>>
>>>>
>>>>
>>>> That's the opposite of what happens. The C frontend makes array element
>>>> types have address-space qualifiers but not the array type.
>>>
>>>
>>> Ah, ok.  Then the opposite way around ;)
>>
>>
>> Ok, so that means that my original patch which updated the element types for
>> arrays is in fact the way to go?
>
> It seems to do both, apply the as to the array _and_ the element type, no?

Yes. I guess I could not do this, but then the patch will also have to 
replace all but very few uses of TYPE_ADDR_SPACE outside the C frontend 
with a new addr_space_for_type function that checks for arrays.

I can do that, but to me it feels like utterly the wrong way to go. If 
you're sure that's what you want, I'll make a patch.


Bernd
Richard Biener Sept. 26, 2014, 4:24 p.m. UTC | #20
On September 26, 2014 5:14:24 PM CEST, Bernd Schmidt <bernds@codesourcery.com> wrote:
>On 09/26/2014 02:42 PM, Richard Biener wrote:
>> On Fri, Sep 26, 2014 at 2:28 PM, Bernd Schmidt
><bernds@codesourcery.com> wrote:
>>> On 09/26/2014 02:26 PM, Richard Biener wrote:
>>>>
>>>> On Fri, Sep 26, 2014 at 2:14 PM, Bernd Schmidt
><bernds@codesourcery.com>
>>>> wrote:
>>>>>
>>>>> On 09/26/2014 02:05 PM, Richard Biener wrote:
>>>>>>
>>>>>>
>>>>>> If currently address-space support matches up with the C frontend
>>>>>> and the C standard then the middle-end has to cope with that.
>>>>>> In this case, cope with array element types not having
>address-space
>>>>>> qualifiers.
>>>>>
>>>>>
>>>>>
>>>>> That's the opposite of what happens. The C frontend makes array
>element
>>>>> types have address-space qualifiers but not the array type.
>>>>
>>>>
>>>> Ah, ok.  Then the opposite way around ;)
>>>
>>>
>>> Ok, so that means that my original patch which updated the element
>types for
>>> arrays is in fact the way to go?
>>
>> It seems to do both, apply the as to the array _and_ the element
>type, no?
>
>Yes. I guess I could not do this, but then the patch will also have to 
>replace all but very few uses of TYPE_ADDR_SPACE outside the C frontend
>
>with a new addr_space_for_type function that checks for arrays.

You have the reference_addr_space function for that.

Richard.

>I can do that, but to me it feels like utterly the wrong way to go. If 
>you're sure that's what you want, I'll make a patch.
>
>
>Bernd
diff mbox

Patch

commit 9a63fbecf0ccf9dd9cf18073958e4cfccf6ecaf2
Author: Bernd Schmidt <bernds@codesourcery.com>
Date:   Wed Sep 10 16:32:27 2014 +0200

    	* tree.c (apply_as_to_type): New function.
    	* tree.h (apply_as_to_type): Declare.

diff --git a/gcc/tree.c b/gcc/tree.c
index d1d67ef..a7438b2 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -6156,6 +6156,21 @@  handle_dll_attribute (tree * pnode, tree name, tree args, int flags,
 
 #endif /* TARGET_DLLIMPORT_DECL_ATTRIBUTES  */
 
+/* Build a type like TYPE, but with address space AS (which can be
+   ADDR_SPACE_GENERIC to remove an existing address space), and return it.  */
+
+tree
+apply_as_to_type (tree type, addr_space_t as)
+{
+  int quals = TYPE_QUALS_NO_ADDR_SPACE (type);
+  if (!ADDR_SPACE_GENERIC_P (as))
+    quals |= ENCODE_QUAL_ADDR_SPACE (as);
+  type = build_qualified_type (type, quals);
+  if (TREE_CODE (type) == ARRAY_TYPE)
+    TREE_TYPE (type) = apply_as_to_type (TREE_TYPE (type), as);
+  return type;
+}
+
 /* Set the type qualifiers for TYPE to TYPE_QUALS, which is a bitmask
    of the various TYPE_QUAL values.  */
 
diff --git a/gcc/tree.h b/gcc/tree.h
index e000e4e..8e1aa6b 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -3845,6 +3845,8 @@  extern tree build_qualified_type (tree, int);
 
 extern tree build_aligned_type (tree, unsigned int);
 
+extern tree apply_as_to_type (tree, addr_space_t);
+
 /* Like build_qualified_type, but only deals with the `const' and
    `volatile' qualifiers.  This interface is retained for backwards
    compatibility with the various front-ends; new code should use