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
