[PATCH] D10972: Add missing builtins to altivec.h for ABI compliance (vol. 3)

[Bill Schmidt]

wschmidt added inline comments.

================
Comment at: lib/Headers/altivec.h:7013
@@ -6712,2 +7012,3 @@
+  return (vector signed long long)__res;
 }
 
----------------
wschmidt wrote:
> nemanjai wrote:
> > wschmidt wrote:
> > > Seems like too much casting above.  It's ok to just use the original code here (and make "vector long long" into "vector unsigned long long").  The instruction will mask off all but the rightmost 6 bits of each element of b, so the signedness doesn't matter.
> > I am not sure if you are referring only to the vector long long overload of vec_sr or to all of them. In any case, the reason for the casts is that Clang will produce an **`lshr`** when the LHS is unsigned and an **`ashr`** when the LHS is signed. This is what was causing LLVM to emit **`vsr[bhwd]`** for the unsigned ones and **`vsra[bhwd]`** for the signed ones. So the casts are simply to ensure that the IR contains the **`lshr`** instructions rather than **`ashr/lshr`** depending on signedness.
> > 
> > Of course, I could have just defined builtins and called the same one for both signed and unsigned overloads, but I assumed that since IR instructions exist for this, it is better to retain the information about what operation is actually being performed in case the optimizer can use it.
> You have wrongly changed the instruction to be generated.  The user has chosen vec_sr, which means the user wants vsr[bhwd].  If they wanted vsra[bhwd], they could have chosen vec_sra.  See figures 4-120 and 4-121 of http://www.freescale.com/files/32bit/doc/ref_manual/ALTIVECPIM.pdf to see the required instruction mappings.
Note that in this case we are adding code for the vector long long operands, which is not described in the PIM.  But we should be consistent with existing behavior for the other types.


Repository:
  rL LLVM

http://reviews.llvm.org/D10972