diff mbox

[ARM] Fix code generation from parsecpu.awk to remove duplicates.

Message ID VI1PR0801MB203191B1657765102CDA6A66FFB80@VI1PR0801MB2031.eurprd08.prod.outlook.com
State New
Headers show

Commit Message

Tamar Christina July 25, 2017, 12:59 p.m. UTC
Hi All,

The new ARM options parser generates from the following entry

begin cpu cortex-a55
 ...
 architecture armv8.2-a+fp16+foo
 option crypto add FP_ARMv8 CRYPTO
 option dotprod add FP_ARMv8 FOO
 option nofp remove ALL_FP
 ...
end cpu cortex-a55

A list of all feature bits that using -mcpu=cortex-a55
enabled and stores these in an array. The size of this array
is statically set to be isa_num_bits which is the last entry
in the isa_feature_bits enum.

What this means is that there is an assumption that you can
never have more feature bits in this array than there are
features.  Which is a reasonable assumption only if the entries
in the array are unique.

The above example currently would add FP_ARMv8 three times.
FP_ARMv8 maps to the macro ISA_FP_ARMv8.  Which when expanded
trivially exceeds the size of the array.

This patch makes sure that when constructing the array we only
emit an a value once. The array is constructed between three
different sources which is where the duplications come in.

Because we can't however look into the macros. When expended
they may still introduce duplicate bits. So this doesn't
completely fix the problem. Just makes it a bit less likely.

Bootstrapped on arm-none-eabi and no issues.

Ok for trunk?

gcc/
2017-07-25  Tamar Christina  <tamar.christina@arm.com>

	* config/arm/parsecpu.awk (all_cores): Remove duplicates.

Comments

Ramana Radhakrishnan July 25, 2017, 1:13 p.m. UTC | #1
On 7/25/17 1:59 PM, Tamar Christina wrote:
> Hi All,
> 
> The new ARM options parser generates from the following entry
> 
> begin cpu cortex-a55
>   ...
>   architecture armv8.2-a+fp16+foo
>   option crypto add FP_ARMv8 CRYPTO
>   option dotprod add FP_ARMv8 FOO
>   option nofp remove ALL_FP
>   ...
> end cpu cortex-a55
> 
> A list of all feature bits that using -mcpu=cortex-a55
> enabled and stores these in an array. The size of this array
> is statically set to be isa_num_bits which is the last entry
> in the isa_feature_bits enum.
> 
> What this means is that there is an assumption that you can
> never have more feature bits in this array than there are
> features.  Which is a reasonable assumption only if the entries
> in the array are unique.
> 
> The above example currently would add FP_ARMv8 three times.
> FP_ARMv8 maps to the macro ISA_FP_ARMv8.  Which when expanded
> trivially exceeds the size of the array.
> 
> This patch makes sure that when constructing the array we only
> emit an a value once. The array is constructed between three
> different sources which is where the duplications come in.
> 
> Because we can't however look into the macros. When expended
> they may still introduce duplicate bits. So this doesn't
> completely fix the problem. Just makes it a bit less likely.
> 
> Bootstrapped on arm-none-eabi and no issues.
> 
> Ok for trunk?
> 
> gcc/
> 2017-07-25  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* config/arm/parsecpu.awk (all_cores): Remove duplicates.
> 


Ok.

Thanks for fixing this.

regards
Ramana
diff mbox

Patch

diff --git a/gcc/config/arm/parsecpu.awk b/gcc/config/arm/parsecpu.awk
index 9d01e2cf992c9643534a7621e59e78041f8a1f9f..070d193b338ace0deb9aeb53a51796bfd633ade8 100644
--- a/gcc/config/arm/parsecpu.awk
+++ b/gcc/config/arm/parsecpu.awk
@@ -223,10 +223,39 @@  function gen_comm_data () {
 	    if (arch_opt_remove[feats[1],feats[m]] == "true") {
 		fatal("cannot remove features from architecture specs")
 	    }
-	    print "        " arch_opt_isa[feats[1],feats[m]] ","
+	    # The isa_features array that is being initialized here has a length
+	    # of max isa_bit_num, which is the last entry in the enum.
+	    # Logically this means that the number of features is implicitly
+	    # never more than the number of feature bits we have.  This is only
+	    # true if we don't emit duplicates here.  So keep track of which
+	    # options we have already emitted so we don't emit them twice.
+	    nopts = split (arch_opt_isa[feats[1],feats[m]], opts, ",")
+	    for (i = 1; i <= nopts; i++) {
+		if (! (opts[i] in seen)) {
+		  print "        " opts[i] ","
+		  seen[opts[i]]
+		}
+	    }
+	}
+	if (cpus[n] in cpu_fpu) {
+	    nopts = split (fpu_isa[cpu_fpu[cpus[n]]], opts, ",")
+	    for (i = 1; i <= nopts; i++) {
+		if (! (opts[i] in seen)) {
+		  print "        " opts[i] ","
+		  seen[opts[i]]
+		}
+	    }
+	}
+	if (cpus[n] in cpu_isa) {
+	    nopts = split (cpu_isa[cpus[n]], opts, ",")
+	    for (i = 1; i <= nopts; i++) {
+		if (! (opts[i] in seen)) {
+		  print "        " opts[i] ","
+		  seen[opts[i]]
+		}
+	    }
 	}
-	if (cpus[n] in cpu_fpu) print "        " fpu_isa[cpu_fpu[cpus[n]]] ","
-	if (cpus[n] in cpu_isa) print "        " cpu_isa[cpus[n]] ","
+	delete seen
 	print "        isa_nobit"
 	print "      }"
 	print "    },"