[PATCH] [IR] Fix the definition of 'shl nsw'

[Sanjoy Das]

================
Comment at: docs/LangRef.rst:5064
@@ +5063,3 @@
+
+If the ``nuw`` keyword is present, then the shift produces a :ref:`poison
+value <poisonvalues>` if ``op1`` \* ``1 << op2`` results in signed overflow.
----------------
Should be ``nsw`` here.

================
Comment at: docs/LangRef.rst:5065
@@ +5064,3 @@
+If the ``nuw`` keyword is present, then the shift produces a :ref:`poison
+value <poisonvalues>` if ``op1`` \* ``1 << op2`` results in signed overflow.
+
----------------
I'd use "if and only if" here.

================
Comment at: docs/LangRef.rst:5068
@@ -5066,2 +5067,3 @@
+As such, NUW/NSW have the same semantics as they
 would if the shift were expressed as a mul instruction with the same
 nsw/nuw bits in (mul %op1, (shl 1, %op2)).
----------------
I'd prefer just removing this, and defining `nuw` directly, like `nsw`.

================
Comment at: docs/LangRef.rst:5080
@@ -5077,3 +5079,3 @@
       <result> = shl i32 1, 32     ; undefined
       <result> = shl <2 x i32> < i32 1, i32 1>, < i32 1, i32 2>   ; yields: result=<2 x i32> < i32 2, i32 4>
 
----------------
Should we add an example of a poison-value creating `shl`?

================
Comment at: lib/Analysis/InstructionSimplify.cpp:2302
@@ +2301,3 @@
+        if (CI2->isMinusOne()) {
+          // 'shl nsw -1, x' produces [-1 << (Width-2), -1]
+          ShiftAmount = Width - 2;
----------------
Is this correct for `i1`?

================
Comment at: test/Transforms/InstCombine/cast.ll:783
@@ -782,3 +782,3 @@
 ; CHECK-LABEL: @test69(
-  %o = shl nsw i64 %i, 3
+  %o = mul nsw i64 %i, 8
   %q = bitcast double* %p to i8*
----------------
Why do we need to change this test?

http://reviews.llvm.org/D8890

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/