diff mbox

[ARM] Fix line number data for PIC register setup code

Message ID 526E2D4A.9030108@mentor.com
State New
Headers show

Commit Message

Tom de Vries Oct. 28, 2013, 9:24 a.m. UTC 
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.

Comments

Eric Botcazou Oct. 29, 2013, 8:12 a.m. UTC | #1 
> 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.

You only asked for approval on trunk though, and I'm not sure we really care 
about the results of the GDB testsuite on the 4.7 branch at this point, so 
let's exclude the 4.7 branch here.
Tom de Vries Oct. 29, 2013, 10:22 a.m. UTC | #2 
On 29/10/13 09:12, Eric Botcazou wrote:
>> 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.
> 
> You only asked for approval on trunk though,

Eric,

Sorry about that.

> and I'm not sure we really care
> about the results of the GDB testsuite on the 4.7 branch at this point,

FWIW, I originally encountered the problem on 4.7.

> so let's exclude the 4.7 branch here.
> 

Reverted on the 4.7 branch.

Thanks,
- Tom
Eric Botcazou Oct. 29, 2013, 11:14 a.m. UTC | #3 
> Reverted on the 4.7 branch.

Thanks.
diff mbox

Patch

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 } } */