Address space information dropped

Hi all,

Tuning my TargetAsmPrinter implementation in the back-end side, I discovered that the address space number is not passed down while emitting global variables with constant initializers. The information is dropped at AsmPrinter::EmitGlobalConstant() function call which defaults it to zero.

I would like to emit target-dependent asm directives depending on the address space of constant initializers. Is there another method to implement this feature ?
The main reason is that we have 2 distinct memories and we are currently using address_space attribute from clang to initialize them.

Thanks in advance,

Ivan

I'm not sure I understand what your problem is. Can you provide an
example of what you see and what you expect, please?

Joerg

Thanks for your quick response Joerg.

We have a very small test case where there is global array and its address space attribute specified like in the following code

const int __attribute__((address_space(256))) fangle[13] =
   {2341, 4681, 7022, 9362, 11703, 1403,16384,
    18725, 21065, 23406, 25746, 28087, 30427};

I need to put its initializer in another memory because it has a different address space and I wanted to pass this information through a target-depend asm directive.
At the moment, I've overridden EmitGlobalVariable() from AsmPrinter because I didn't see any other spot to put this special directive.

Ivan

Have you thought about always tagging such global variables with
__attribute__((section("fancyaddresspace"))) as well? That seems to be
the sanest way I can think of.

Joerg

This sounds like a bug.

-Eli

Thanks for your quick response Joerg.

We have a very small test case where there is global array and its
address space attribute specified like in the following code

const int __attribute__((address_space(256))) fangle[13] =
   {2341, 4681, 7022, 9362, 11703, 1403,16384,
    18725, 21065, 23406, 25746, 28087, 30427};

I need to put its initializer in another memory because it has a
different address space and I wanted to pass this information
through a target-depend asm directive.
At the moment, I've overridden EmitGlobalVariable() from AsmPrinter
because I didn't see any other spot to put this special directive.

Have you thought about always tagging such global variables with
__attribute__((section("fancyaddresspace"))) as well? That seems to be
the sanest way I can think of.

Yes, but it seems a little odd to me. Is it correct to assume that different sections imply different address spaces ?
I rather wanted to have a special asm directive on the top of section definitions to switch between memories following an identical section layout for both of them.

Ivan

>>Thanks for your quick response Joerg.
>>
>>We have a very small test case where there is global array and its
>>address space attribute specified like in the following code
>>
>>const int __attribute__((address_space(256))) fangle[13] =
>> {2341, 4681, 7022, 9362, 11703, 1403,16384,
>> 18725, 21065, 23406, 25746, 28087, 30427};
>>
>>I need to put its initializer in another memory because it has a
>>different address space and I wanted to pass this information
>>through a target-depend asm directive.
>>At the moment, I've overridden EmitGlobalVariable() from AsmPrinter
>>because I didn't see any other spot to put this special directive.
>Have you thought about always tagging such global variables with
>__attribute__((section("fancyaddresspace"))) as well? That seems to be
>the sanest way I can think of.

Yes, but it seems a little odd to me. Is it correct to assume that
different sections imply different address spaces ?
I rather wanted to have a special asm directive on the top of
section definitions to switch between memories following an
identical section layout for both of them.

Let's take a step back. "address space" is a type modifier like __thread.

From codegen perspective, primarily changes the way access is done.

Variable declaration requires obtaining some pointer value and possibly
initialising it. For __thread, this is done by putting the object into
.tdata or .tbss sections and letting the linker do the rest. That is:
- merging corresponding sections
- ensuring that (in cooperation with the runtime linker and thread
library) initialisation happens as needed
- correct relocation records are created.

What needs to happen in your use case is essentially the same. You don't
need some of the complications of __thread, but the idea is:
(1) Put the objects into a separate section.
(2) Make sure that the section ends up in your memory image by using an
appropiate linker script.
(3) Make sure that the section is relocated to a fixed location,
representing the offset in the address space.
(4) Add startup code to copy the content of the executable to the
address space.

At least for an ELF target or comparable object format, you don't need
special assembler support.

Joerg

Thanks for your quick response Joerg.

We have a very small test case where there is global array and its
address space attribute specified like in the following code

const int __attribute__((address_space(256))) fangle[13] =
   {2341, 4681, 7022, 9362, 11703, 1403,16384,
    18725, 21065, 23406, 25746, 28087, 30427};

I need to put its initializer in another memory because it has a
different address space and I wanted to pass this information
through a target-depend asm directive.
At the moment, I've overridden EmitGlobalVariable() from AsmPrinter
because I didn't see any other spot to put this special directive.

Have you thought about always tagging such global variables with
__attribute__((section("fancyaddresspace"))) as well? That seems to be
the sanest way I can think of.

Yes, but it seems a little odd to me. Is it correct to assume that
different sections imply different address spaces ?
I rather wanted to have a special asm directive on the top of
section definitions to switch between memories following an
identical section layout for both of them.

Let's take a step back. "address space" is a type modifier like __thread.

From codegen perspective, primarily changes the way access is done.

Variable declaration requires obtaining some pointer value and possibly
initialising it. For __thread, this is done by putting the object into
.tdata or .tbss sections and letting the linker do the rest. That is:
- merging corresponding sections
- ensuring that (in cooperation with the runtime linker and thread
library) initialisation happens as needed
- correct relocation records are created.

Thanks for your clear explanation.

What needs to happen in your use case is essentially the same. You don't
need some of the complications of __thread, but the idea is:
(1) Put the objects into a separate section.
(2) Make sure that the section ends up in your memory image by using an
appropiate linker script.
(3) Make sure that the section is relocated to a fixed location,
representing the offset in the address space.
(4) Add startup code to copy the content of the executable to the
address space.

At least for an ELF target or comparable object format, you don't need
special assembler support.

Looks like a cleaner solution, thanks!

I know MCELFStreamer has default sections for every kind of data to stream out. I thought I could save some efforts if I reused them, and its section flags also, avoiding the specification of additional ones.
Nevertheless, it's clearly not a linker-friendly solution...

Ivan

Hi Eli,

Hi all,

Tuning my TargetAsmPrinter implementation in the back-end side, I
discovered that the address space number is not passed down while
emitting global variables with constant initializers. The information is
dropped at AsmPrinter::EmitGlobalConstant() function call which defaults
it to zero.

This sounds like a bug.

The feature added to allow targets emit custom data directives based on the address space seems to be broken. However, it doesn't produce any odd behavior because it's not used.
The attached patch is quite small and shows where the address space information is getting lost and it will correctly propagate it. It also avoids existent back-ends to break because of an unimplemented hook in MCAsmInfo.

Ivan

addrspace-asmprinter.patch (2.03 KB)

Ivan,

The attached patch is quite small and shows where the address space
information is getting lost and it will correctly propagate it. It also
avoids existent back-ends to break because of an unimplemented hook in
MCAsmInfo.

Testcase, please, if possible :slight_smile:

@@ -1046,7 +1046,8 @@
       if (CPE.isMachineConstantPoolEntry())
         EmitMachineConstantPoolValue(CPE.Val.MachineCPVal);
       else
- EmitGlobalConstant(CPE.Val.ConstVal);
+ EmitGlobalConstant(CPE.Val.ConstVal, (isa<PointerType>(Ty)) ?
+ cast<PointerType>(Ty)->getAddressSpace() : 0);
     }
   }
}

This looks suspicious; how can an entry in the constant pool possibly
acquire an address space based on the type of the value?

-Eli

Hi Eli,

Hi all,

Tuning my TargetAsmPrinter implementation in the back-end side, I
discovered that the address space number is not passed down while
emitting global variables with constant initializers. The information is
dropped at AsmPrinter::EmitGlobalConstant() function call which defaults
it to zero.

This sounds like a bug.

The feature added to allow targets emit custom data directives based on the
address space seems to be broken. However, it doesn't produce any odd
behavior because it's not used.
The attached patch is quite small and shows where the address space
information is getting lost and it will correctly propagate it. It also
avoids existent back-ends to break because of an unimplemented hook in
MCAsmInfo.

@@ -1046,7 +1046,8 @@
        if (CPE.isMachineConstantPoolEntry())
          EmitMachineConstantPoolValue(CPE.Val.MachineCPVal);
        else
- EmitGlobalConstant(CPE.Val.ConstVal);
+ EmitGlobalConstant(CPE.Val.ConstVal, (isa<PointerType>(Ty)) ?
+ cast<PointerType>(Ty)->getAddressSpace() : 0);
      }
    }
  }

This looks suspicious; how can an entry in the constant pool possibly
acquire an address space based on the type of the value?

IIUC, global values are also constants, I added that verification just in case. But I agree that it's may be unrealistic.

Ivan

Hi Anton,

No back-end uses this information, so I don't know if it's worth to put this upstream. Following the approach that Joerg described very well, a section change and a linker script can take care of it, instead of customizing basic data directives based on the address space. It turns to be sane and good enough for me.

Ivan

Hi Ivan,

IIUC, global values are also constants,

the address of a global is a constant, but the contents are not necessarily
constant. (Since a GlobalVariable represents the address of the variable it
holds, your statement is literally correct, but maybe that's not what you
meant).

Ciao, Duncan.

  I added that verification just