[PATCH] D127546: [GISel] Fix narrowScalar() for big endian targets

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added a subscriber: 0x59616e.
myhsu added a comment.

Thank you for sending this patch, it's always good to see love on big endian support in GlobalISel.

Though the thing you try to fix here seems to be more complicated than it looked. First, this is the MIR output of `constant.ll` **before** applying your patch:

  body:             |
    bb.1 (%ir-block.0):
      %3:_(s32) = G_CONSTANT i32 51966
      %4:_(s32) = G_CONSTANT i32 0
      $d1 = COPY %3(s32)
      $d0 = COPY %4(s32)
      RTS implicit $d1, implicit $d0

Here is the assembly code:

  ; %bb.0:
          move.l  #51966, %d1
          move.l  #0, %d0
          rts

This is actually more or less what we want. The reason I say "more or less" is because m68k doesn't really specify the calling convention of returning 64-bit integers (at least I don't find any in SysV ABI). For Linux/GCC ABI, which is the primary ABI we're supporting now, it puts Lo part in `$d1` and Hi part in `$d0`, same as the snippet above. Here is the assembly code if you compile the equivalent C code using `m68k-linux-gnu-gcc-9`:

  link.w %fp,#0
  sub.l %a0,%a0
  move.l #51966,%a1
  move.l %a0,%d0
  move.l %a1,%d1
  unlk %fp
  rts

But here is the catch: the ordering shown above was a product of D116877 <https://reviews.llvm.org/D116877> by @0x59616e, which honored the endianness when splitting a 64-bit value. In other words, we only start to worry about endianness of integer parts (e.g. Hi and Lo parts) upon materializing them into register pairs (endianness is meaningless when it's not stored in memory, but register pairs that carry integer parts are special cases where people usually put parts in registers that follows endianness).
However, after this patch is applied, it effectively cancels out D116877 <https://reviews.llvm.org/D116877> because integer parts are swapped again.

So now the question is: who is responsible for taking care of endianness when it comes to integer parts? In SelectionDAGISel, particularly its type legalizer, such task is mostly discussed case-by-case: vector types definitely needs to be aware of endianness, some operations like bitcast also needs to do byte swapping if needed. But I don't see similar things happen on constant (link <https://github.com/llvm/llvm-project/blob/2d2da259c8726fd5c974c01122a9689981a12196/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp#L3200>). Another place that considers endianness is SelecitonDAGBuilder while it's generating Copy nodes (e.g. CopyToReg), which is similar to D116877 <https://reviews.llvm.org/D116877> where we take care of endianness on integer parts upon materializing them into physical registers.

The bottom line is that maybe this is not the right place to reorder narrowed integer parts according to endianness. But I'm genuinely becoming more confused by this seemly simple endianness issue so it will be great if folks here who are more familiar with this topic can give some comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127546/new/

https://reviews.llvm.org/D127546