This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: improved RTL-level if conversion using scratchpads [half-hammock edition]
- From: Bernd Schmidt <bschmidt at redhat dot com>
- To: Abe <abe_skolnik at yahoo dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Sebastian Pop <sebpop at gmail dot com>, Kyrill Tkachov <kyrylo dot tkachov at arm dot com>
- Date: Fri, 6 Nov 2015 11:56:08 +0100
- Subject: Re: improved RTL-level if conversion using scratchpads [half-hammock edition]
- Authentication-results: sourceware.org; auth=none
- References: <563BE9A7 dot 30803 at yahoo dot com>
On 11/06/2015 12:43 AM, Abe wrote:
Feedback from Bernd has also been applied.
But inconsistently, and I think not quite in the way I meant it in one case.
-/* Return true if a write into MEM may trap or fault. */
-
static bool
noce_mem_write_may_trap_or_fault_p (const_rtx mem)
{
- rtx addr;
-
if (MEM_READONLY_P (mem))
return true;
- if (may_trap_or_fault_p (mem))
- return true;
-
+ rtx addr;
addr = XEXP (mem, 0);
/* Call target hook to avoid the effects of -fpic etc.... */
@@ -2881,6 +2883,18 @@ noce_mem_write_may_trap_or_fault_p (const_rtx mem)
return false;
}
+/* Return true if a write into MEM may trap or fault
+ without scratchpad support. */
+
+static bool
+unsafe_address_p (const_rtx mem)
+{
+ if (may_trap_or_fault_p (mem))
+ return true;
+
+ return noce_mem_write_may_trap_or_fault_p (mem);
+}
The naming seems backwards from what I suggested in terms of naming. You
still haven't explained why you want to modify this function, or call
the limited one even when generating scratchpads. I asked about this
last time.
static basic_block
-find_if_header (basic_block test_bb, int pass)
+find_if_header (basic_block test_bb, int pass, bool just_sz_spad)
{
Arguments need documentation.
+DEFPARAM (PARAM_FORCE_ENABLE_RTL_IFCVT_SPADS,
+ "force-enable-rtl-ifcvt-spads",
+ "Force-enable the use of scratchpads in RTL if conversion, "
+ "overriding the target and the profile data or lack thereof.",
+ 0, 0, 1)
+
+DEFHOOKPOD
+(rtl_ifcvt_scratchpad_control,
+"*",
+enum rtl_ifcvt_spads_ctl_enum, rtl_ifcvt_spads_as_per_profile)
+
+DEFHOOK
+(rtl_ifcvt_get_spad,
+ "*",
+ rtx, (unsigned short size),
+ default_rtl_ifcvt_get_spad)
That moves the problematic bit in a target hook rather than fixing it.
Two target hooks and a param is probably a bit much for a change like
this. Which target do you actually want this for? It would help to
understand why you're doing all this.
+enum rtl_ifcvt_spads_ctl_enum {
Names are still too verbose ("enum" shouldn't be part of it).
+rtx default_rtl_ifcvt_get_spad (unsigned short size)
+{
+ return assign_stack_local (BLKmode, size, 0);
+}
Formatting problem, here and in a few other places. I didn't fully read
the patch this time around.
I'm probably not reviewing further patches because I don't see this
progressing to a state where it's acceptable. Others may do so, but as
far as I'm concerned the patch is rejected.
Bernd