Re: [PATCH, ARM] Fix line number data for PIC register setup code

[Tom de Vries <Tom_deVries at mentor dot com>]

On 27/10/13 17:04, Eric Botcazou wrote:
>> The PIC register setup code is emitted after NOTE_INSNS_FUNCTION_BEG,
>> because it uses the insert_insn_on_edge mechanism, and the corresponding
>> insertion in cfgexpand.c:gimple_expand_cfg takes care to insert the code
>> after the parm_birth_insn:
>> ...
>> 	      /* Avoid putting insns before parm_birth_insn.  */
>> 	      if (e->src == ENTRY_BLOCK_PTR
>> 		  && single_succ_p (ENTRY_BLOCK_PTR)
>> 		  && parm_birth_insn)
>> 		{
>> 		  rtx insns = e->insns.r;
>> 		  e->insns.r = NULL_RTX;
>> 		  emit_insn_after_noloc (insns, parm_birth_insn, e->dest);
>> 		}
>> ...
>> And in the case for this test-case, parm_birth_insn is the
>> NOTE_INSNS_FUNCTION_BEG.
> 
> So this means that parm_birth_insn can never be null, right?
> 

Yes, AFAICT parm_birth_insn is never NULL at this point.

>> 2013-10-13  Tom de Vries  <tom@codesourcery.com>
>>
>> 	* cfgexpand.c (gimple_expand_cfg): Don't commit insertions after
>> 	NOTE_INSN_FUNCTION_BEG.
>>
>> 	* gcc.target/arm/require-pic-register-loc.c: New test.
> 
> OK if you also remove the test on parm_birth_insn.
> 

Updated patch, re-bootstrapped on x86_64 and committed to trunk.

Also applied to 4.7 and 4.8 branches, the same problem is present there.

Thanks,
- Tom

2013-10-28  Tom de Vries  <tom@codesourcery.com>

	* cfgexpand.c (gimple_expand_cfg): Remove test for parm_birth_insn.
	Don't commit insertions after NOTE_INSN_FUNCTION_BEG.

	* gcc.target/arm/require-pic-register-loc.c: New test.

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index ba4c0e6..9705036 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -4789,14 +4789,18 @@ gimple_expand_cfg (void)
 	  if (e->insns.r)
 	    {
 	      rebuild_jump_labels_chain (e->insns.r);
-	      /* Avoid putting insns before parm_birth_insn.  */
+	      /* Put insns after parm birth, but before
+		 NOTE_INSNS_FUNCTION_BEG.  */
 	      if (e->src == ENTRY_BLOCK_PTR
-		  && single_succ_p (ENTRY_BLOCK_PTR)
-		  && parm_birth_insn)
+		  && single_succ_p (ENTRY_BLOCK_PTR))
 		{
 		  rtx insns = e->insns.r;
 		  e->insns.r = NULL_RTX;
-		  emit_insn_after_noloc (insns, parm_birth_insn, e->dest);
+		  if (NOTE_P (parm_birth_insn)
+		      && NOTE_KIND (parm_birth_insn) == NOTE_INSN_FUNCTION_BEG)
+		    emit_insn_before_noloc (insns, parm_birth_insn, e->dest);
+		  else
+		    emit_insn_after_noloc (insns, parm_birth_insn, e->dest);
 		}
 	      else
 		commit_one_edge_insertion (e);
 2013-10-27  Tobias Burnus  <burnus@net-b.de>
 
 	PR other/33426
diff --git a/gcc/testsuite/gcc.target/arm/require-pic-register-loc.c b/gcc/testsuite/gcc.target/arm/require-pic-register-loc.c
new file mode 100644
index 0000000..bd85e86
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/require-pic-register-loc.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-g -fPIC" } */
+
+void *v;
+void a (void *x) { }
+void b (void) { }
+                       /* line 7.  */
+int                    /* line 8.  */
+main (int argc)        /* line 9.  */
+{                      /* line 10.  */
+  if (argc == 12345)   /* line 11.  */
+    {
+      a (v);
+      return 1;
+    }
+  b ();
+
+  return 0;
+}
+
+/* { dg-final { scan-assembler-not "\.loc 1 7 0" } } */
+/* { dg-final { scan-assembler-not "\.loc 1 8 0" } } */
+/* { dg-final { scan-assembler-not "\.loc 1 9 0" } } */
+
+/* The loc at the start of the prologue.  */
+/* { dg-final { scan-assembler-times "\.loc 1 10 0" 1 } } */
+
+/* The loc at the end of the prologue, with the first user line.  */
+/* { dg-final { scan-assembler-times "\.loc 1 11 0" 1 } } */