<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<div class="moz-cite-prefix">On 07/11/2017 12:54 AM, Dylan McKay via
llvm-dev wrote:<br>
</div>
<blockquote
cite="mid:CAMizNnv+SKAzg1NDiKo92X==03C1vXWOCrv+9fDPt=+yZEhrmQ@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<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
moz-do-not-send="true"
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>
</div>
</blockquote>
<br>
Hi, Dylan,<br>
<br>
Being able to specify the address space of functions, etc. is a good
idea. Given the current design, you can't put this into
TargetTransformInfo, however, because nothing in TTI may be required
for correctness (because your target's implementation might not be
available). Information required for correctness must go in
DataLayout (because it must always be available). You should propose
patches to add this information to DataLayout and to use that
information in relevant places.<br>
<br>
-Hal<br>
<br>
<blockquote
cite="mid:CAMizNnv+SKAzg1NDiKo92X==03C1vXWOCrv+9fDPt=+yZEhrmQ@mail.gmail.com"
type="cite">
<div dir="ltr">
<div class="markdown-here-wrapper" style="">
<div
title="MDH:SGVsbG8gYWxsLCBJJ20gbG9va2luZyBpbnRvIHNvbHZpbmcgYW4gQVZSLXNwZWNpZmljIGlzc3VlIGFuZCB3b3VsZCBsb3ZlIHRvIGhlYXIgcGVvcGxlcyB0aG91Z2h0cyBvbiBob3cgdG8gYmVzdCBm
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>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
LLVM Developers mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</body>
</html>