<div dir="ltr"><div class="markdown-here-wrapper" style=""><p style="margin:0px 0px 1.2em!important">Hello all, I’m looking into solving an AVR-specific issue and would love to hear peoples thoughts on how to best fix it.</p>
<h1 id="background" style="margin:1.3em 0px 1em;padding:0px;font-weight:bold;font-size:1.6em;border-bottom:1px solid rgb(221,221,221)">Background</h1>
<p style="margin:0px 0px 1.2em!important">As you may or may not know, I maintain the in-tree AVR backend, which also happens to be (to the best of my knowledge) the first in-tree backend for a Harvard architecture.</p>
<p style="margin:0px 0px 1.2em!important">In this architecture, code lives inside the ‘program memory’ space (numbered 1), whereas data lives inside RAM “data space”, which corresponds to the default address space 0. This is important because loads/stores use different instruction/pointer formats depending on the address space used, and so we need correct address space information available to the backend itself.</p>
<p style="margin:0px 0px 1.2em!important">Due to the fact that address spaces in LLVM default to 0, this means that all global or constant variables default to living inside data space. This causes a few issues, including the fact that the SimplifyCFG pass creates switch lookup tables, which default to data space, causing us to emit broken table lookups and also wasting precious RAM.</p>
<h1 id="the-problem-emitting-pointers-as-operands" style="margin:1.3em 0px 1em;padding:0px;font-weight:bold;font-size:1.6em;border-bottom:1px solid rgb(221,221,221)">The problem - emitting pointers as operands</h1>
<p style="margin:0px 0px 1.2em!important"><strong>NOTE</strong>: Feel free to skip to tl;dr of this section if you don’t care too much about the details</p>
<p style="margin:0px 0px 1.2em!important">There are different instructions which require different fixups to be applied depending on whether pointers are located in data space or program space.</p>
<p style="margin:0px 0px 1.2em!important">Take the <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">ICALL</code> instruction - it performs an indirect call to the pointer stored in the <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">Z</code> register.</p>
<p style="margin:0px 0px 1.2em!important">We must first load the pointer into <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">Z</code> via the ‘ldi’ instruction. If the pointer is actually a pointer to a symbol, we need to emit a <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">AVR_LO8_LDI_GS</code> relocation, otherwise we emit a <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">AVR_LO8_LDI</code> relocation. There are a few other cases, but they’re irrelevant for this discussion.</p>
<p style="margin:0px 0px 1.2em!important">We can quite easily look at the <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">GlobalValue*</code> that corresponds to the pointer if it is a symbol and select the fixup based on that, but that assumes that the address spaces are always correct.</p>
<p style="margin:0px 0px 1.2em!important">Now, imagine that the pointer is actually a function pointer. LLVM does not expose any way to set address space in the IR for functions, but because it derived from GlobalValue, it does have an address space, and that address space defaults to zero. Because of this, every single function pointer in the AVR backend that gets loaded by the <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">ldi</code> will be associated with data space, and not program space, which it actually belongs to.</p>
<p style="margin:0px 0px 1.2em!important"><strong>tl;dr</strong> functions default to address space zero, even though they are in a different space on Harvard architectures, which causes silent codegen bugs when we rely on the address space of a global value</p>
<h1 id="proposed-solution" style="margin:1.3em 0px 1em;padding:0px;font-weight:bold;font-size:1.6em;border-bottom:1px solid rgb(221,221,221)">Proposed solution</h1>
<p style="margin:0px 0px 1.2em!important">It would be impossible to set the address space correctly on creation of <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">llvm::Function</code> objects because at that point in the pipeline, we do not know the target architecture.</p>
<p style="margin:0px 0px 1.2em!important">Because of this, I’d like to extend <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">TargetTransformInfo</code> with hooks that like <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">getSwitchTableAddressSpace()</code>, <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">getFunctionAddressSpace()</code>. I have already got a WIP patch for this <a href="https://reviews.llvm.org/D34983">here</a>.</p>
<p style="margin:0px 0px 1.2em!important">Once we have that information available to TargetTransformInfo, I propose we add a pass (very early in the codegen pipeline) that sets the address space of all functions to whatever value is specified in the hooks.</p>
<p style="margin:0px 0px 1.2em!important">This works well because we don’t let frontends specify address space on functions, nor do we even mention that functions have address spaces in the language reference.</p>
<p style="margin:0px 0px 1.2em!important">The downside of it it is that you wouldn’t normally expect something like an address space to change midway through the compilation process. To counter that however, I doubt the pre-codegen code cares much about the value of function address spaces, if at all.</p>
<p style="margin:0px 0px 1.2em!important">On top of this, at the current point in time, <code style="font-size:0.85em;font-family:Consolas,Inconsolata,Courier,monospace;margin:0px 0.15em;padding:0px 0.3em;white-space:pre-wrap;border:1px solid rgb(234,234,234);background-color:rgb(248,248,248);border-radius:3px;display:inline">Pointer<Function>::getAddressSpace</code> is downright incorrect on any Harvard architecture, and for other architectures, the address space for functions will still stay the default of zero and will not change at all.</p>
<p style="margin:0px 0px 1.2em!important">Does anybody know anything I haven’t thought of? Any reasons why this solution is suboptimal?</p>
<div title="MDH:SGVsbG8gYWxsLCBJJ20gbG9va2luZyBpbnRvIHNvbHZpbmcgYW4gQVZSLXNwZWNpZmljIGlzc3Vl
IGFuZCB3b3VsZCBsb3ZlIHRvIGhlYXIgcGVvcGxlcyB0aG91Z2h0cyBvbiBob3cgdG8gYmVzdCBm
aXggaXQuPGRpdj48YnI+PC9kaXY+PGRpdj4jIEJhY2tncm91bmQ8YnI+PGRpdj48YnI+PC9kaXY+
PGRpdj5BcyB5b3UgbWF5IG9yIG1heSBub3Qga25vdywgSSBtYWludGFpbiB0aGUgaW4tdHJlZSBB
VlIgYmFja2VuZCwgd2hpY2ggYWxzbyBoYXBwZW5zIHRvIGJlICh0byB0aGUgYmVzdCBvZiBteSBr
bm93bGVkZ2UpIHRoZSBmaXJzdCBpbi10cmVlIGJhY2tlbmQgZm9yIGEgSGFydmFyZCBhcmNoaXRl
Y3R1cmUuPC9kaXY+PGRpdj48YnI+PC9kaXY+PGRpdj5JbiB0aGlzIGFyY2hpdGVjdHVyZSwgY29k
ZSBsaXZlcyBpbnNpZGUgdGhlICdwcm9ncmFtIG1lbW9yeScgc3BhY2UgKG51bWJlcmVkIDEpLCB3
aGVyZWFzIGRhdGEgbGl2ZXMgaW5zaWRlIFJBTSAiZGF0YSBzcGFjZSIsIHdoaWNoIGNvcnJlc3Bv
bmRzIHRvIHRoZSBkZWZhdWx0IGFkZHJlc3Mgc3BhY2UgMC4gVGhpcyBpcyBpbXBvcnRhbnQgYmVj
YXVzZSBsb2Fkcy9zdG9yZXMgdXNlIGRpZmZlcmVudCBpbnN0cnVjdGlvbi9wb2ludGVyIGZvcm1h
dHMgZGVwZW5kaW5nIG9uIHRoZSBhZGRyZXNzIHNwYWNlIHVzZWQsIGFuZCBzbyB3ZSBuZWVkIGNv
cnJlY3QgYWRkcmVzcyBzcGFjZSBpbmZvcm1hdGlvbiBhdmFpbGFibGUgdG8gdGhlIGJhY2tlbmQg
aXRzZWxmLjwvZGl2PjxkaXY+PGJyPjwvZGl2PjxkaXY+RHVlIHRvIHRoZSBmYWN0IHRoYXQgYWRk
cmVzcyBzcGFjZXMgaW4gTExWTSBkZWZhdWx0IHRvIDAsIHRoaXMgbWVhbnMgdGhhdCBhbGwgZ2xv
YmFsIG9yIGNvbnN0YW50IHZhcmlhYmxlcyBkZWZhdWx0IHRvIGxpdmluZyBpbnNpZGUgZGF0YSBz
cGFjZS4gVGhpcyBjYXVzZXMgYSBmZXcgaXNzdWVzLCBpbmNsdWRpbmcgdGhlIGZhY3QgdGhhdCB0
aGUgU2ltcGxpZnlDRkcgcGFzcyBjcmVhdGVzIHN3aXRjaCBsb29rdXAgdGFibGVzLCB3aGljaCBk
ZWZhdWx0IHRvIGRhdGEgc3BhY2UsIGNhdXNpbmcgdXMgdG8gZW1pdCBicm9rZW4gdGFibGUgbG9v
a3VwcyBhbmQgYWxzbyB3YXN0aW5nIHByZWNpb3VzIFJBTS48L2Rpdj48L2Rpdj48ZGl2Pjxicj48
L2Rpdj48ZGl2PiMgVGhlIHByb2JsZW0gLSBlbWl0dGluZyBwb2ludGVycyBhcyBvcGVyYW5kczwv
ZGl2PjxkaXY+PGJyPjwvZGl2PjxkaXY+KipOT1RFKio6IEZlZWwgZnJlZSB0byBza2lwIHRvIHRs
O2RyIG9mIHRoaXMgc2VjdGlvbiBpZiB5b3UgZG9uJ3QgY2FyZSBhYm91dCB0aGUgZGV0YWlsczwv
ZGl2PjxkaXY+PGJyPjwvZGl2PjxkaXY+VGhlcmUgYXJlIGRpZmZlcmVudCBpbnN0cnVjdGlvbnMg
d2hpY2ggcmVxdWlyZSBkaWZmZXJlbnQgZml4dXBzIHRvIGJlIGFwcGxpZWQgZGVwZW5kaW5nIG9u
IHdoZXRoZXIgcG9pbnRlcnMgYXJlIGxvY2F0ZWQgaW4gZGF0YSBzcGFjZSBvciBwcm9ncmFtIHNw
YWNlLjwvZGl2PjxkaXY+PGJyPjwvZGl2PjxkaXY+VGFrZSB0aGUgYElDQUxMYCBpbnN0cnVjdGlv
biAtIGl0IHBlcmZvcm1zIGFuIGluZGlyZWN0IGNhbGwgdG8gdGhlIHBvaW50ZXIgc3RvcmVkIGlu
IHRoZSBgWmAgcmVnaXN0ZXIuPC9kaXY+PGRpdj48YnI+PC9kaXY+PGRpdj5XZSBtdXN0IGZpcnN0
IGxvYWQgdGhlIHBvaW50ZXIgaW50byBgWmAgdmlhIHRoZSAnbGRpJyBpbnN0cnVjdGlvbi4gSWYg
dGhlIHBvaW50ZXIgaXMgYWN0dWFsbHkgYSBwb2ludGVyIHRvIGEgc3ltYm9sLCB3ZSBuZWVkIHRv
IGVtaXQgYSBgQVZSX0xPOF9MRElfR1NgIHJlbG9jYXRpb24sIG90aGVyd2lzZSB3ZSBlbWl0IGEg
YEFWUl9MTzhfTERJYCByZWxvY2F0aW9uLiBUaGVyZSBhcmUgYSBmZXcgb3RoZXIgY2FzZXMsIGJ1
dCB0aGV5J3JlIGlycmVsZXZhbnQgZm9yIHRoaXMgZGlzY3Vzc2lvbi48L2Rpdj48ZGl2Pjxicj48
L2Rpdj48ZGl2PldlIGNhbiBxdWl0ZSBlYXNpbHkgbG9vayBhdCB0aGUgYEdsb2JhbFZhbHVlKmAg
dGhhdCBjb3JyZXNwb25kcyB0byB0aGUgcG9pbnRlciBpZiBpdCBpcyBhIHN5bWJvbCBhbmQgc2Vs
ZWN0IHRoZSBmaXh1cCBiYXNlZCBvbiB0aGF0LCBidXQgdGhhdCBhc3N1bWVzIHRoYXQgdGhlIGFk
ZHJlc3Mgc3BhY2VzIGFyZSBhbHdheXMgY29ycmVjdC48L2Rpdj48ZGl2Pjxicj48L2Rpdj48ZGl2
Pk5vdywgaW1hZ2luZSB0aGF0IHRoZSBwb2ludGVyIGlzIGFjdHVhbGx5IGEgZnVuY3Rpb24gcG9p
bnRlci4gTExWTSBkb2VzIG5vdCBleHBvc2UgYW55IHdheSB0byBzZXQgYWRkcmVzcyBzcGFjZSBp
biB0aGUgSVIgZm9yIGZ1bmN0aW9ucywgYnV0IGJlY2F1c2UgaXQgZGVyaXZlZCBmcm9tIEdsb2Jh
bFZhbHVlLCBpdCBkb2VzIGhhdmUgYW4gYWRkcmVzcyBzcGFjZSwgYW5kIHRoYXQgYWRkcmVzcyBz
cGFjZSBkZWZhdWx0cyB0byB6ZXJvLiBCZWNhdXNlIG9mIHRoaXMsIGV2ZXJ5IHNpbmdsZSBmdW5j
dGlvbiBwb2ludGVyIGluIHRoZSBBVlIgYmFja2VuZCB0aGF0IGdldHMgbG9hZGVkIGJ5IHRoZSBg
bGRpYCB3aWxsIGJlIGFzc29jaWF0ZWQgd2l0aCBkYXRhIHNwYWNlLCBhbmQgbm90IHByb2dyYW0g
c3BhY2UsIHdoaWNoIGl0IGFjdHVhbGx5IGJlbG9uZ3MgdG8uPC9kaXY+PGRpdj48YnI+PC9kaXY+
PGRpdj4qKnRsO2RyKiogZnVuY3Rpb25zIGRlZmF1bHQgdG8gYWRkcmVzcyBzcGFjZSB6ZXJvLCBl
dmVuIHRob3VnaCB0aGV5IGFyZSBpbiBhIGRpZmZlcmVudCBzcGFjZSBvbiBIYXJ2YXJkIGFyY2hp
dGVjdHVyZXMsIHdoaWNoIGNhdXNlcyBzaWxlbnQgY29kZWdlbiBidWdzIHdoZW4gd2UgcmVseSBv
biB0aGUgYWRkcmVzcyBzcGFjZSBvZiBhIGdsb2JhbCB2YWx1ZTwvZGl2PjxkaXY+PGJyPjwvZGl2
PjxkaXY+IyBQcm9wb3NlZCBzb2x1dGlvbjwvZGl2PjxkaXY+PGJyPjwvZGl2PjxkaXY+SXQgd291
bGQgYmUgaW1wb3NzaWJsZSB0byBzZXQgdGhlIGFkZHJlc3Mgc3BhY2UgY29ycmVjdGx5IG9uIGNy
ZWF0aW9uIG9mIGBsbHZtOjpGdW5jdGlvbmAgb2JqZWN0cyBiZWNhdXNlIGF0IHRoYXQgcG9pbnQg
aW4gdGhlIHBpcGVsaW5lLCB3ZSBkbyBub3Qga25vdyB0aGUgdGFyZ2V0IGFyY2hpdGVjdHVyZS48
L2Rpdj48ZGl2Pjxicj48L2Rpdj48ZGl2PkJlY2F1c2Ugb2YgdGhpcywgSSdkIGxpa2UgdG8gZXh0
ZW5kIGBUYXJnZXRUcmFuc2Zvcm1JbmZvYCB3aXRoIGhvb2tzIHRoYXQgbGlrZSBgZ2V0U3dpdGNo
VGFibGVBZGRyZXNzU3BhY2UoKWAsIGBnZXRGdW5jdGlvbkFkZHJlc3NTcGFjZSgpYC4gSSBoYXZl
IGFscmVhZHkgZ290IGEgV0lQIHBhdGNoIGZvciB0aGlzIFtoZXJlXShodHRwczovL3Jldmlld3Mu
bGx2bS5vcmcvRDM0OTgzKS48L2Rpdj48ZGl2Pjxicj48L2Rpdj48ZGl2Pk9uY2Ugd2UgaGF2ZSB0
aGF0IGluZm9ybWF0aW9uIGF2YWlsYWJsZSB0byBUYXJnZXRUcmFuc2Zvcm1JbmZvLCBJIHByb3Bv
c2Ugd2UgYWRkIGEgcGFzcyAodmVyeSBlYXJseSBpbiB0aGUgY29kZWdlbiBwaXBlbGluZSkgdGhh
dCBzZXRzIHRoZSBhZGRyZXNzIHNwYWNlIG9mIGFsbCBmdW5jdGlvbnMgdG8gd2hhdGV2ZXIgdmFs
dWUgaXMgc3BlY2lmaWVkIGluIHRoZSBob29rcy48L2Rpdj48ZGl2Pjxicj48L2Rpdj48ZGl2PlRo
aXMgd29ya3Mgd2VsbCBiZWNhdXNlIHdlIGRvbid0IGxldCBmcm9udGVuZHMgc3BlY2lmeSBhZGRy
ZXNzIHNwYWNlIG9uIGZ1bmN0aW9ucywgbm9yIGRvIHdlIGV2ZW4gbWVudGlvbiB0aGF0IGZ1bmN0
aW9ucyBoYXZlIGFkZHJlc3Mgc3BhY2VzIGluIHRoZSBsYW5ndWFnZSByZWZlcmVuY2UuPC9kaXY+
PGRpdj48YnI+PC9kaXY+PGRpdj5UaGUgZG93bnNpZGUgb2YgaXQgaXQgaXMgdGhhdCB5b3Ugd291
bGRuJ3Qgbm9ybWFsbHkgZXhwZWN0IHNvbWV0aGluZyBsaWtlIGFuIGFkZHJlc3Mgc3BhY2UgdG8g
Y2hhbmdlIG1pZHdheSB0aHJvdWdoIHRoZSBjb21waWxhdGlvbiBwcm9jZXNzLiBUbyBjb3VudGVy
IHRoYXQgaG93ZXZlciwgSSBkb3VidCB0aGUgcHJlLWNvZGVnZW4gY29kZSBjYXJlcyBtdWNoIGFi
b3V0IHRoZSB2YWx1ZSBvZiBmdW5jdGlvbiBhZGRyZXNzIHNwYWNlcywgaWYgYXQgYWxsLjwvZGl2
PjxkaXY+PGJyPjwvZGl2PjxkaXY+T24gdG9wIG9mIHRoaXMsIGF0IHRoZSBjdXJyZW50IHBvaW50
IGluIHRpbWUsIGBQb2ludGVyJmx0O0Z1bmN0aW9uJmd0Ozo6Z2V0QWRkcmVzc1NwYWNlYCBpcyBk
b3ducmlnaHQgaW5jb3JyZWN0IG9uIGFueSBIYXJ2YXJkIGFyY2hpdGVjdHVyZSwgYW5kIGZvciBv
dGhlciBhcmNoaXRlY3R1cmVzLCB0aGUgYWRkcmVzcyBzcGFjZSBmb3IgZnVuY3Rpb25zIHdpbGwg
c3RpbGwgc3RheSB0aGUgZGVmYXVsdCBvZiB6ZXJvIGFuZCB3aWxsIG5vdCBjaGFuZ2UgYXQgYWxs
LjwvZGl2PjxkaXY+PGJyPjwvZGl2PjxkaXY+RG9lcyBhbnlib2R5IGtub3cgYW55dGhpbmcgSSBo
YXZlbid0IHRob3VnaHQgb2Y/IEFueSByZWFzb25zIHdoeSB0aGlzIHNvbHV0aW9uIGlzIHN1Ym9w
dGltYWw/PC9kaXY+" style="height:0;width:0;max-height:0;max-width:0;overflow:hidden;font-size:0em;padding:0;margin:0">​</div></div></div>