<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 04/25/2014 10:19 AM, Filip Pizlo
wrote:<br>
</div>
<blockquote cite="mid:etPan.535a9915.419ac241.2885@dethklok.local"
type="cite">
<style>body{font-family:Helvetica,Arial;font-size:13px}</style>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<br>
<p style="color:#000;">On April 25, 2014 at 9:52:35 AM, Eric
Christopher (<a moz-do-not-send="true"
href="mailto:echristo@gmail.com">echristo@gmail.com</a>)
wrote:</p>
<div>
<blockquote type="cite" class="clean_bq" style="color: rgb(0, 0,
0); font-family: Helvetica, Arial; font-size: 13px;
font-style: normal; font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: normal; orphans: auto;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255,
255, 255);"><span>
<div>
<div>Hi Michael,<br>
<br>
> I’d like to propose to extend LLVM IR intrinsics
set, adding new ones for<br>
> safe-division. There are intrinsics for detecting
overflow errors, like<br>
> sadd.with.overflow, and the intrinsics I’m
proposing will augment this set.<br>
><br>
> The new intrinsics will return a structure with two
elements according to<br>
> the following rules:<br>
><br>
> safe.[us]div(x,0) = safe.[us]rem(x,0) = {0, 1}<br>
> safe.sdiv(min<T>, -1) =
safe.srem(min<T>, -1) = {min<T>, 1}<br>
> In other cases: safe.op(x,y) = {x op y, 0}, where
op is sdiv, udiv, srem, or<br>
> urem<br>
><br>
><br>
> The use of these intrinsics would be quite the same
as it was for<br>
> arith.with.overflow intrinsics. For instance:<br>
> %res = call {i32, i1} @llvm.safe.sdiv.i32(i32 %a,
i32 %b)<br>
> %div = extractvalue {i32, i1} %res, 0<br>
> %bit = extractvalue {i32, i1} %res, 1<br>
> br i1 %bit, label %trap, label %normal<br>
><br>
<br>
I notice that the patch is still in ToT even though
we're now having a<br>
discussion on whether it should go in. It should be
reverted.<br>
<br>
That said, I don't see how this is anything except for
an optimization<br>
intrinsic and not necessary for correct behavior for any
language.<br>
I.e. You can do what the PNaCl people are doing and emit
branches<br>
instead. Since this will only happen on a single target
and not on all<br>
targets why not just make this a target intrinisic? I
don't buy the<br>
argument that it biases the target independent IR since
any pass that<br>
uses TTI in the IR level does the same.</div>
</div>
</span></blockquote>
</div>
<p>The sdiv operation in LLVM IR only makes sense for C and its
very direct relatives. The amount of control flow necessary to
represent a safe division for any other language is ghastly.
(a/b) becomes something like (b != 0 ? ((a != INT_MIN || b !=
-1) ? a / b : INT_MIN) : 0). It's more useful to the optimizer
to see this as the single operation that it really is.</p>
<p>Also, this isn't about just a single target. Any target that
has to emulate integer division (like a lot of embedded systems
would do) would benefit from this since the division emulation
can most naturally be written to provide the "safe" semantics
rather than LLVM's default semantics (i.e. undefined behavior on
x/0 and INT_MIN/-1).</p>
<p>I think adding these intrinics to the IR would be a great help
for non-C clients of LLVM.</p>
<p>-Filip<br>
</p>
</blockquote>
I agree with all of Filip's points above. I'm in support of
including these intrinsics in their current form.<br>
<br>
I have two suggestions, but both of these can be incremental
improvements:<br>
1) On many targets, it might make sense to lower these much earlier
than codegenprepare. By lowering them that late, you loose a lot of
potentially to have branches eliminated. If there's no special
hardware support which makes those conditions cheap, getting rid of
them early is probably the best choice. <br>
2) Extending Reid's suggestion (in a separate response) could we
define the intrinsics to take the (constant) values to be used in
the boundary conditions? llvm.safe.div(x,y, undef, undef) would
give Reid's desired semantics. llvm.safe.div(x,y, 0, min<T>)
would give the original proposed semantics. This would allow a
variety of language semantics with a shared implementation. <br>
<br>
Philip<br>
</body>
</html>