[PATCH] D129291: [NVPTX] Promote i24, i40, i48 and i56 to next power-of-two register when passing

Fri Jul 15 12:35:42 PDT 2022

tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM with one test nit.

In D129291#3654804 <https://reviews.llvm.org/D129291#3654804>, @kjetilkjeka wrote:

>> I would stick with a set of `CHECK-DAG: ld.param.XX {{.*}} [test_i23_param_0+Y]`. All we care about here is that we load the right set of bits. We can assume that reconstructing the integer is handled by appropriate tests already.
>
> I realize that you say that all we care about is loading the right set of bits. But I assume we would like to check it for returns/args on both the caller and callee side? These four cases is basically what I'm doing now in addition to checking that the function is actually called. I have removed the checking of truncating/promoting as you mentioned.

Let me rephrase -- we only care about loads/stores from the param space. That does apply to both load of parameters from the function arguments and storing them to pass parameters and return values. 
I think we are in agreement on what we need to check. The comment was largely about checks for `cvt`, `shl` and `or` instructions we match (e.g a few still remain in `test_i24`).

================
Comment at: llvm/test/CodeGen/NVPTX/param-load-store.ll:567
+; CHECK:      ld.param.u8     [[E8:%r[0-9]+]], [test_i24_param_0+2];
+; CHECK:      shl.b32         [[S16:%r[0-9]+]], [[E8]], 16;
+; CHECK:      ld.param.u16    [[E16:%r[0-9]+]], [test_i24_param_0];
----------------
we want CHECK-DAG on loads here.

Nit: no need to check logical ops. 

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

https://reviews.llvm.org/D129291