[PATCH] D123973: [AMDGPU] Split the lit test spill-vgpr-to-agpr.ll to different tests

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

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

In D123973#3460775 <https://reviews.llvm.org/D123973#3460775>, @hsmhsm wrote:

> In D123973#3459961 <https://reviews.llvm.org/D123973#3459961>, @rampitec wrote:
>
>> It does not seem to move tests but rather dropping them.
>
> I cross-checked it again to verify if I have accidentally removed any tests - but it does not seem so, the changes reflect what I mentioned in the review message.
>
> In detail - without this change - the file spill-vgpr-to-agpr.ll contains following functions which are tested against gfx900 ang gfx908.
>
> [1]. @max_10_vgprs()
> [2]. @max_10_vgprs_used_9a
> [3]. @max_10_vgprs_used_1a_partial_spill
> [4]. @max_10_vgprs_spill_v32
> [5]. @max_256_vgprs_spill_9x32
> [6]. @max_256_vgprs_spill_9x32_2bb
> [7]. @stack_args_vgpr_spill
>
> For gfx900, while testing @max_10_vgprs_used_9a and/or @max_10_vgprs_used_1a_partial_spill, the compilation fails because of `a` constraint.  Remaining functions passes compilation for gfx900. 
> For gfx908, compilation passes for all 7 functions.
>
> Now within this change - as discussed in the https://reviews.llvm.org/D123525, I have moved the check for error on  `agpr` constraint in case of gfx900 to a new file - reject-agpr-usage-before-gfx908.ll
>
> And with this, both @max_10_vgprs_used_9a and @max_10_vgprs_used_1a_partial_spill are really to be tested only against gfx908, and remaining 5 functions to be tested against both gf900 and gfx908.
>
> Ans, I did so by keeping both @max_10_vgprs_used_9a and @max_10_vgprs_used_1a_partial_spill within the original file spill-vgpr-to-agpr.ll and testing it only against gfx908, and by moving all other 5 functions to a new file spill-vgpr.ll and testing all five of them against both gfx900 and gfx908.
>
> Note that in the original file, we do few GCN checks for @max_10_vgprs_used_9a and @max_10_vgprs_used_1a_partial_spill against gfx900 even though the compilation fails (this is possible because of the quirk nature of llc which I explained in https://reviews.llvm.org/D123525), which are now got omitted since we now test these two functions only against gfx908. And, it actually does not make sense at all to test for assembly when the compilation itself fails, which is rather very confusing thing.
>
> With this understanding, I am still not able to get - what test did I drop while doing the change as I mentioned above?

Ugh. You have copied the file! OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123973