[PATCH] D132910: [SimplifyCFG] add a test for branch folding multiple BB

Yaxun Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 14:15:05 PDT 2022


yaxunl marked an inline comment as done.
yaxunl added inline comments.


================
Comment at: llvm/test/Transforms/SimplifyCFG/branch-fold-multiple.ll:5
+
+target triple = "amdgcn-amd-amdhsa"
+
----------------
yaxunl wrote:
> fhahn wrote:
> > Does this need to be target specific? We should definitely have at least some target-independent tests here.
> The cost of bonus instructions depends on the target triple and processor (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L3676). Certain processors have high costs than others, causing the branch not to be folded.
> 
> For example, if I replace the target triple of this lit test with `x86_64` and remove the target processor. This lit test still passes since `x86_64` behaves similarly as amdgcn gfx906. However, if I replace the target triple with `x86`, this lit test will fail since the branches are never folded even without my patch.
> 
> How about I make the IR in this lit test target independent, and use `-mtriple` to test a few triples which are relevant to the test?
> The cost of bonus instructions depends on the target triple and processor (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L3676). Certain processors have high costs than others, causing the branch not to be folded.
> 
> For example, if I replace the target triple of this lit test with `x86_64` and remove the target processor. This lit test still passes since `x86_64` behaves similarly as amdgcn gfx906. However, if I replace the target triple with `x86`, this lit test will fail since the branches are never folded even without my patch.
> 
> How about I make the IR in this lit test target independent, and use `-mtriple` to test a few triples which are relevant to the test?

The drawback of the above approach is that it requires multiple registered targets, which can make it less tested on bots.

Another way is to use the existing option for bonus insts threshold. Increase it so that the lit test behaves the same on all targets. Then this lit test will be target independent.


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

https://reviews.llvm.org/D132910



More information about the llvm-commits mailing list