<html>
    <head>
      <base href="https://bugs.llvm.org/">
    </head>
    <body><table border="1" cellspacing="0" cellpadding="8">
        <tr>
          <th>Bug ID</th>
          <td><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - convert set-clear bit using select to bit math"
   href="https://bugs.llvm.org/show_bug.cgi?id=37581">37581</a>
          </td>
        </tr>

        <tr>
          <th>Summary</th>
          <td>convert set-clear bit using select to bit math
          </td>
        </tr>

        <tr>
          <th>Product</th>
          <td>libraries
          </td>
        </tr>

        <tr>
          <th>Version</th>
          <td>trunk
          </td>
        </tr>

        <tr>
          <th>Hardware</th>
          <td>PC
          </td>
        </tr>

        <tr>
          <th>OS</th>
          <td>All
          </td>
        </tr>

        <tr>
          <th>Status</th>
          <td>NEW
          </td>
        </tr>

        <tr>
          <th>Severity</th>
          <td>enhancement
          </td>
        </tr>

        <tr>
          <th>Priority</th>
          <td>P
          </td>
        </tr>

        <tr>
          <th>Component</th>
          <td>Common Code Generator Code
          </td>
        </tr>

        <tr>
          <th>Assignee</th>
          <td>unassignedbugs@nondot.org
          </td>
        </tr>

        <tr>
          <th>Reporter</th>
          <td>spatel+llvm@rotateright.com
          </td>
        </tr>

        <tr>
          <th>CC</th>
          <td>llvm-bugs@lists.llvm.org
          </td>
        </tr></table>
      <p>
        <div>
        <pre>Filing this based on code in <a href="https://reviews.llvm.org/D47323">https://reviews.llvm.org/D47323</a> (although the
patch itself has nothing to do with this problem).

We have bit magic source like this in the LLVM optimizer:

unsigned setAllowReassoc(unsigned Flags, bool B) {
  return (Flags & ~8) | B * 8;
}

That shouldn't be necessary. This code is functionally equivalent and
(probably?) easier to understand:

unsigned setAllowReassocIf(unsigned Flags, bool B) {
  return B ? Flags | 8 : Flags & ~8;
}

The resulting x86 asm is quite different though:

$ clang++ bitmagic.c -O2 -S -o - 
        andl    $-9, %edi
        leal    (%rdi,%rsi,8), %eax
        retq

vs.     
        movl    %edi, %eax
        orl     $8, %eax
        andl    $-9, %edi
        testl   %esi, %esi
        cmovnel %eax, %edi
        movl    %edi, %eax
        retq

----------------------------------------------------------------------------

We should decide on the canonical IR:

  %and = and i32 %Flags, -9
  %conv = zext i1 %B to i32
  %mul = shl nuw nsw i32 %conv, 3
  %r = or i32 %mul, %and
=>
  %or = or i32 %Flags, 8
  %and2 = and i32 %Flags, -9
  %r = select i1 %B, i32 %or, i32 %and2


If we choose the select form (since it has less instructions), then this is a
codegen bug first. If we choose the bit-ops, then this is probably just an
instcombine bug (but we need to test codegen on other targets to make sure that
asm is better on non-x86 too).

This bug is present in clang 6.0, so it's not a result of:
<a href="https://reviews.llvm.org/D46086">https://reviews.llvm.org/D46086</a>

That patch demonstrates possible trade-offs in our IR decision: bitops may seem
like the right choice, but they can make other analysis more difficult.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are on the CC list for the bug.</li>
      </ul>
    </body>
</html>