[PATCH] D109940: Apply proper source location to fallthrough switch cases

[Adrian Prantl via Phabricator via llvm-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:1523
+  // by a default statement.
+  if (CurCase->getSubStmt()->getStmtClass() == Stmt::DefaultStmtClass) {
+    EmitStopPoint(CurCase);
----------------
rastogishubham wrote:
> aprantl wrote:
> > Is the default statement special in a meaningful way here, or could a more general patch also work for a switch statement like:
> > ```
> >   switch (num) {
> >   case 0:
> >     break;
> >   case 10: // break here
> >   case 11:
> >      break;
> >   default:
> > }
> > ```
> > 
> > In other words, should wee just unconditionally emit a stop point?
> So if you have a fallthrough to another case statement, clang doesn't generate a branch just like it does for the default, for example:
> 
> void func(int num) {
>     switch (num) {
>     case 0:
>         break;
>     case 10: // break here
>     case 11:
>         break;
>     default:
>         break;
>     }
> }
> 
> dasm:
> 
> tmp0:
> 	.loc	1 2 13 prologue_end             ## switchFallthruCaseStmt.c:2:13
> 	movl	-4(%rbp), %eax
> 	.loc	1 2 5 is_stmt 0                 ## switchFallthruCaseStmt.c:2:5
> 	testl	%eax, %eax
> 	movl	%eax, -8(%rbp)                  ## 4-byte Spill
> 	je	LBB0_1
> 	jmp	LBB0_5
> LBB0_5:
> 	.loc	1 0 5                           ## switchFallthruCaseStmt.c:0:5
> 	movl	-8(%rbp), %eax                  ## 4-byte Reload
> 	.loc	1 2 5                           ## switchFallthruCaseStmt.c:2:5
> 	addl	$-10, %eax
> 	subl	$2, %eax
> 	jb	LBB0_2
> 	jmp	LBB0_3
> LBB0_1:
> Ltmp1:
> 	.loc	1 4 9 is_stmt 1                 ## switchFallthruCaseStmt.c:4:9
> 	jmp	LBB0_4
> LBB0_2:
> 	.loc	1 7 9                           ## switchFallthruCaseStmt.c:7:9
> 	jmp	LBB0_4
> LBB0_3:
> 	.loc	1 9 9                           ## switchFallthruCaseStmt.c:9:9
> 	jmp	LBB0_4
> Ltmp2:
> LBB0_4:
> 	.loc	1 11 1                          ## switchFallthruCaseStmt.c:11:1
> 	popq	%rbp
> 	retq
> 
> If you notice, clang has basically mashed the case 10 and 11 together and not generated a jump from case 10 to case 11. So I think it only happens when there is a case statement followed by a default.
> So if you have a fallthrough to another case statement, clang doesn't generate a branch just like it does for the default, for example:

Interesting. Thanks!


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

https://reviews.llvm.org/D109940