[PATCH] D22696: [CodeGenPrep] Skip merging empty case blocks

[Renato Golin via llvm-commits]

rengolin added a comment.

The change kind of makes sense to me, but I'd rather others with more experience on basic block merging to opine first.

I've added a couple of points inline. It would be good to see this behaviour in other targets, though.

It'd also be good to do a run on the test-suite to make sure there's no regressions (maybe code size?) in at least one target.

cheers,
--renato


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:426
@@ +425,3 @@
+        if (PN->getBasicBlockIndex(BB) >= 0) {
+          IsIncomingBlock = true;
+          break;
----------------
Why not just continue here?

================
Comment at: test/CodeGen/AArch64/aarch64-skip-merging-case-block.ll:8
@@ +7,3 @@
+
+; CHECK: bl checkNum
+; CHECK: ldr x[[JT:[0-9]+]], [x{{[0-9]+}}, x{{[0-9]+}}, lsl #3]
----------------
Please, put the check lines where the related sources are, down there with the IR.

It's not clear how this lines test anything you implemented above.

================
Comment at: test/CodeGen/AArch64/aarch64-skip-merging-case-block.ll:22
@@ +21,3 @@
+
+;int F1(const char* a, const char* b, int len) ;
+;extern unsigned char PL1[];
----------------
This is a very convoluted test. Can't you make it simpler?

>From the description in the comments, you just need a switch in a loop, and you don't need any of those structs, or extern functions or anything.

================
Comment at: test/CodeGen/AArch64/aarch64-skip-merging-case-block.ll:164
@@ +163,3 @@
+
+declare i32 @checkNum(...) local_unnamed_addr #1
+
----------------
Can you simplify your code to not need those functions? The change comment implies you can...

================
Comment at: test/CodeGen/AArch64/aarch64-skip-merging-case-block.ll:172
@@ +171,3 @@
+
+attributes #0 = { nounwind "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="kryo" "target-features"="+crc,+crypto,+neon" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="kryo" "target-features"="+crc,+crypto,+neon" "unsafe-fp-math"="false" "use-soft-float"="false" }
----------------
Do you really need the attributes to make the test exhibit the behaviour you're trying to fix?


https://reviews.llvm.org/D22696