<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 19 June 2014 02:56, Dinesh Dwivedi <span dir="ltr"><<a href="mailto:dinesh.d@samsung.com" target="_blank">dinesh.d@samsung.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Nick,<br>
<br>
m_Not checks for pattern XOR(X, -1) and will not work in current scenario.<br>
i.e. it can check if we have XOR(X, -1) pattern but can not identify that<br>
0x55555555 and ~(0xAAAAAAAA) are NOT of each other.<br>
<br>
Am I missing something?<br></blockquote><div><br></div><div>Sounds like m_Not is missing something. Could I interest you in teaching it to handle m_Not(ConstantInt) by looking for ~CI?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div class=""><br>
Regards<br>
Dinesh Dwivedi<br>
<br>
------- Original Message -------<br>
</div>Sender : Nick Lewycky<<a href="mailto:nicholas@mxc.ca">nicholas@mxc.ca</a>><br>
Date   : Jun 19, 2014 11:50 (GMT+05:30)<br>
<div class="im HOEnZb">Title  : Re: [PATCH] Added instruction combine to transform few more negative<br>
 values addition to subtraction<br>
<br>
</div><div class="HOEnZb"><div class="h5">Nick Lewycky wrote:<br>
> Sadness. Did you know that gcc gets this even at -O0? Not this pattern mind you, but the original testcase. It's that simple, at least before we transform to xor(or) form. GCC doesn't get it in xor(or) form either.<br>


><br>
> I looked for a better way to address this optimizer deficiency and didn't find one. The one thing I didn't check is whether we could get it by reordering our optimizations inside instcombine and do something else before it gets into xor(or) form. Even if that did fix the testcase in PR14365 it would still leave the case where someone actually does write "((a|~c) ^ c) + (a+1)".<br>


<br>
Actually, I was just thinking about this some more. There's no reason<br>
'C' needs to be a constant at all, is there? As long as it's X and ~X<br>
(use m_Value(X) then m_Not(m_Specific(X))) the transform should be<br>
correct, right?<br>
<br>
Nick<br>
<br>
><br>
> ================<br>
> Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:928<br>
> @@ +927,3 @@<br>
> +<br>
> +  // This function creates 2 instructions to replace ADD, we need atleast one of<br>
> +  // LHS or RHS to have one use to ensure benefit in transform.<br>
> ----------------<br>
> "atleast" -->  "at least"<br>
><br>
> ================<br>
> Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:951<br>
> @@ +950,3 @@<br>
> +    if (match(X, m_Xor(m_Value(Y), m_APInt(C1)))) {<br>
> +      if (match(Y, m_Or(m_Value(Z), m_APInt(C2)))&&  (*C2 == ~(*C1))) {<br>
> +        Value *NewAnd = Builder->CreateAnd(Z, *C1);<br>
> ----------------<br>
> Optional: *C2 == ~(*C1) might be more efficient as !C2->intersects(*C1). It isn't, but in theory we could improve intersects to not compute an intermediate value (ie., it doesn't need to look at all the bits once it finds a pair are set in both).<br>


><br>
> ================<br>
> Comment at: test/Transforms/InstCombine/add2.ll:142<br>
> @@ +141,1 @@<br>
> +}<br>
>  No newline at end of file<br>
><br>
> ----------------<br>
> Please make sure there's a newline at the end of the file.<br>
><br>
> <a href="http://reviews.llvm.org/D3733" target="_blank">http://reviews.llvm.org/D3733</a><br>
><br>
><br>
><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>