<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Strong -1 here. This seems like an abuse of bitcast to avoid
nailing down interfaces and semantics for restricted address space
casts. IMO, we should not accept the proposed IR change and
should instead work on standardizing APIs/semantics for restricted
addrspacecasts. </p>
<p><br>
</p>
<p>Philip<br>
</p>
<p><br>
</p>
<div class="moz-cite-prefix">On 11/25/21 2:50 AM, Sankisa, Krishna
(Chaitanya) via llvm-dev wrote:<br>
</div>
<blockquote type="cite"
cite="mid:SN6PR12MB26230D8ADFAFAF0C0E3C74C4FB629@SN6PR12MB2623.namprd12.prod.outlook.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<style type="text/css" style="display:none;">P {margin-top:0;margin-bottom:0;}</style>
<p
style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;"
align="Left">
[AMD Official Use Only]<br>
</p>
<br>
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif;
font-size: 12pt; color: rgb(0, 0, 0);">
TL;DR
<div>=====</div>
<div><br>
</div>
<div>We propose the following change to LLVM IR:</div>
<div>- Allow bitcast to support no-op pointer cast between
pointers from different address spaces.</div>
<div>- This bitcast is valid if the bit widths queried for
addressspaces from datalayout match.</div>
<div>- Overload CastIsValid call with datalayout argument to
check validity of cast.</div>
<div>- Update CastIsValid to allow bitcast between vector of
pointers from different address spaces if total bit widths
match.</div>
<div>- GVN pass introduces ptrtoint/inttoptr for load which
reinterprets bits from previous store.
</div>
<div> Instead use a no-op bitcast of ptrs from different
address spaces.</div>
<div><br>
</div>
<div><br>
</div>
<div>Motivation</div>
<div>==========</div>
<div><br>
</div>
<div>When addrspacecast was introduced, abilty to do no-op
pointer bitcast from different address spaces has been
removed.</div>
<div>Pointer sizes are always known from DataLayout which is
now made mandatory in LLVM IR.
</div>
<div>So, Bitcast can be analysed to be no-op cast by matching
the pointer sizes from DataLayout.</div>
<div><br>
</div>
<div>Since there is no other way to do no-op reinterpret of
bits, in some cases GVN pass introduces a ptrtoint/inttoptr
pair.</div>
<div>After proper analysis, that a no-op bitcast can be done
is concluded, then a bitcast can be introduced.</div>
<div>Usage of no-op pointer bitcast between addrspaces can be
restricted to be used only by IR Transform passes but not by
frontend.</div>
<div><br>
</div>
<div>For example consider the below IR:</div>
<div>GVN pass has discovered a reinterpretation of bits via a
store followed by a load.</div>
<div><br>
</div>
<div>%struct.S.coerce = type { i32 addrspace(1)* }</div>
<div>%s.sroa.0 = alloca i32*, align 8, addrspace(5)</div>
<div>%0 = extractvalue %struct.S.coerce %s.coerce, 0</div>
<div>%1 = bitcast i32* addrspace(5)* %s.sroa.0 to i32
addrspace(1)* addrspace(5)*</div>
<div>%2 = addrspacecast i32 addrspace(1)* addrspace(5)* %1 to
i32 addrspace(1)</div>
<div>store i32 addrspace(1)* %0, i32 addrspace(1) %2, align 8</div>
<div><br>
</div>
<div>%3 = load i32*, i32* addrspace(5)* %s.sroa.0, align 8,
!tbaa !2</div>
<div><br>
</div>
<div>;GVN pass currently introduces no-op ptrotoint/inttoptr
for load.</div>
<div>%3 = ptrtoint i32 addrspace(1)* %0 to i64</div>
<div>%4 = inttoptr i64 %3 to i32*</div>
</div>
</div>
</blockquote>
<p>Honestly, this really looks like a bug in GVN. We could
reasonable insert an *addrspacecast*, but the round trip through
integer values seems highly suspect.</p>
<p><br>
</p>
<p>Also, a key missing detail in your example is that you've taught
your alias analysis to peak through addrspacecasts. I don't
believe the default implementation will conclude that %s.sroa.0
and %2 are mustalias. Nothing wrong with it, just noting it due
to the potential confusion. <br>
</p>
<blockquote type="cite"
cite="mid:SN6PR12MB26230D8ADFAFAF0C0E3C74C4FB629@SN6PR12MB2623.namprd12.prod.outlook.com">
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif;
font-size: 12pt; color: rgb(0, 0, 0);">
<div><br>
</div>
<div>;If bitcast of pointers from different address is
allowed, load can be replaced with no-op bitcast</div>
<div>%3 = bitcast i32 addrspace(1)* %0 to i32*</div>
<div><br>
</div>
<div><br>
</div>
<div>Implementation</div>
<div>==============</div>
<div><br>
</div>
<div>1. There are certain cases where standalone instructions
are created without linking them to basicblock/function or
module.
</div>
<div> In such cases DataLayout is not accessible by querying
the module. To check validity of bitcast datalayout is
mandatory.</div>
<div> So CastInst::CastIsValid, CastInst::create etc have
been overloaded to take DataLayout as argument.</div>
<div><br>
</div>
<div> static bool castIsValid(Instruction::CastOps op, Value
*S, Type *DstTy,</div>
<div> const DataLayout &DL);</div>
<div><br>
</div>
<div> static CastInst *Create(</div>
<div> Instruction::CastOps, ///< The opcode of the
cast instruction</div>
<div> Value *S, ///< The value to be
casted (operand 0)</div>
<div> Type *Ty, ///< The type to which
cast should be made</div>
<div> const DataLayout &DL, ///< DataLayout to
check validity of bitcast</div>
<div> const Twine &Name = "", ///< Name for the
instruction</div>
<div> Instruction *InsertBefore = nullptr ///< Place
to insert the instruction</div>
<div> );</div>
<div><br>
</div>
<div>2. Verifier has been updated to check for validity of
bitcast using datalayout.</div>
<div><br>
</div>
<div>3. GVN pass has been updated to use bitcast for a load
instead of emitting ptrtoint/<span
style="background-color:rgb(255, 255, 255);display:inline
!important"><span> </span>inttoptr.</span></div>
<div> llvm/lib/Transforms/Utils/VNCoercion.cpp</div>
<div><br>
</div>
<div>Review link: <a href="https://reviews.llvm.org/D114533"
id="LPlnkOWALinkPreview" moz-do-not-send="true">
https://reviews.llvm.org/D114533</a> </div>
<div class="_Entity _EType_OWALinkPreview _EId_OWALinkPreview
_EReadonly_1">
<div
id="LPBorder_GTaHR0cHM6Ly9yZXZpZXdzLmxsdm0ub3JnL0QxMTQ1MzM."
class="LPBorder988765" style="width: 100%; margin-top:
16px; margin-bottom: 16px; position: relative; max-width:
800px; min-width: 424px;">
<table id="LPContainer988765" role="presentation"
style="padding: 12px 36px 12px 12px; width: 100%;
border-width: 1px; border-style: solid; border-color:
rgb(200, 200, 200); border-radius: 2px;">
<tbody>
<tr style="border-spacing: 0px;" valign="top">
<td style="width: 100%;">
<div id="LPTitle988765" style="font-size: 21px;
font-weight: 300; margin-right: 8px;
font-family: wf_segoe-ui_light, "Segoe UI
Light", "Segoe WP Light",
"Segoe UI", "Segoe WP",
Tahoma, Arial, sans-serif; margin-bottom: 12px;">
<a target="_blank" id="LPUrlAnchor988765"
href="https://reviews.llvm.org/D114533"
style="text-decoration: none; color:
var(--themePrimary);" moz-do-not-send="true">⚙
D114533 LLVM IR should allow bitcast between
address spaces with the same size.</a></div>
<div id="LPDescription988765" style="font-size:
14px; max-height: 100px; color: rgb(102, 102,
102); font-family: wf_segoe-ui_normal,
"Segoe UI", "Segoe WP",
Tahoma, Arial, sans-serif; margin-bottom: 12px;
margin-right: 8px; overflow: hidden;">
LLVM IR should allow bitcast between address
spaces with the same size.</div>
<div id="LPMetadata988765" style="font-size: 14px;
font-weight: 400; color: rgb(166, 166, 166);
font-family: wf_segoe-ui_normal, "Segoe
UI", "Segoe WP", Tahoma, Arial,
sans-serif;">
reviews.llvm.org</div>
</td>
</tr>
</tbody>
</table>
</div>
</div>
<div><br>
</div>
<div>Regards,</div>
Chaitanya<br>
</div>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<pre class="moz-quote-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="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
</body>
</html>