Fix #3561: assert on RyuJIT x86 when generating shl by 1

This assert hit in the encoder when we were trying to generate an
INS_shl_N with a constant of 1, instead of using the special xarch
INS_shl_1 encoding, which saves a byte. It turns out, the assert was
and in fact amd64 does generate the suboptimal encoding currently.

The bad code occurs in the RMW case of genCodeForShift(). It turns out
that function is unnecessarily complex, unique (it doesn't use the
common RMW code paths), and has a number of other latent bugs.

To fix this, I split genCodeForShift() by leaving the non-RMW case
there, and adding a genCodeForShiftRMW() function just for the RMW case.
I rewrote the RMW case to use the existing emitInsRMW functions.

Other related cleanups along the way:
1. I changed emitHandleMemOp to consistently set the idInsFmt field,
and changed all callers to stop pre-setting or post-setting this field.
This makes the API much easier to understand. I added a big header
comment for the function. Now, it takes a "basic" insFmt (using ARD,
AWR, or ARW forms), which might be munged to a MRD/MWR/MRW form
if necessary.
2. I changed some places to always use the most derived GenTree type
for all uses. For instance, if the code has
"GenTreeIndir* mem = node->AsIndir()", then always use "mem" from then
on, and don't use "node". I changed some functions to take more derived
GenTree node types.
3. I rewrote the emitInsRMW() functions to be much simpler, and rewrote
their header comments.
4. I added GenTree predicates OperIsShift(), OperIsRotate(), and
OperIsShiftOrRotate().
5. I added function genMapShiftInsToShiftByConstantIns() to encapsulate
mapping from INS_shl to INS_shl_N or INS_shl_1 based on a constant.
This code was in 3 different places already.
6. The change in assertionprop.cpp is simply to make JitDumps readable.

In addition to fixing the bug for RyuJIT/x86, there are a small number
of x64 diffs where we now generate smaller encodings for shift by 1.

1 files changed, 3 insertions, 1 deletions

diff --git a/src/jit/emit.h b/src/jit/emit.h
index 39241278a5..95dac33536 100644
--- a/src/jit/emit.h
+++ b/src/jit/emit.h
@@ -1511,7 +1511,9 @@ private:
     regNumber            emitInsBinary  (instruction ins, emitAttr attr, GenTree* dst, GenTree* src);
     regNumber            emitInsTernary (instruction ins, emitAttr attr, GenTree* dst, GenTree* src1, GenTree* src2);
     void                 emitInsMov(instruction ins, emitAttr attr, GenTree *node);
-    void                 emitHandleMemOp(GenTree *mem, instrDesc *id, bool isSrc);
+    insFormat            emitMapFmtForIns(insFormat fmt, instruction ins);
+    insFormat            emitMapFmtAtoM(insFormat fmt);
+    void                 emitHandleMemOp(GenTreeIndir* indir, instrDesc* id, insFormat fmt, instruction ins);
     void                 spillIntArgRegsToShadowSlots();
 #endif // !LEGACY_BACKEND