On 3/31/25 12:39, Jeff Law wrote:
>>>> * config/riscv/riscv-vsetvl.cc (pre_vsetvl::emit_vsetvl): skip
>>>> EDGE_ABNORMAL.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> * go.dg/pr119533-riscv.go: New test.
>>> So presumably it wants to insert on the EH edge for a reason. Just
>>> skipping the edge is probably wrong.
>>>
>>> The way these scenarios are handled in the more generic LCM passes is to
>>> kill all values on the EH edge. With all values killed on the EH edge,
>>> no redundancy exists which should require insertion on the edge.
>> Lets do some dump diving - pun intended.
>>
>> Failure happens at the following abnormal edge:
>>
>> Insert missed vsetvl info at edge(bb 42 -> bb 64):VALID (insn 663, bb
>> 64)
>>
>>
>> Where bb 64 corresponds to unwinder call so clearly EH related.
>>
>> (code_label/s 474 591 476 64 37 (nil) [1 uses])
>> (note 476 474 438 64 [bb 64] NOTE_INSN_BASIC_BLOCK)
>> (call_insn 438 476 439 64 (parallel [
>> (call (mem:SI (symbol_ref:DI ("_Unwind_Resume") [flags 0x41]
>> <function_decl 0x7ef116e59400 __builtin_unwind_resume>) [0
>> __builtin_unwind_resume S4 A32])
>> (const_int 0 [0]))
>> (use (unspec:SI [
>> (const_int 0 [0])
>> ] UNSPEC_CALLEE_CC))
>> (clobber (reg:SI 1 ra))
>> ]) 456 {call_internal}
>> (expr_list:REG_DEAD (reg:DI 10 a0)
>> (expr_list:REG_CALL_DECL (symbol_ref:DI ("_Unwind_Resume")
>> [flags
>> 0x41] <function_decl 0x7ef116e59400 __builtin_unwind_resume>)
>> (expr_list:REG_NORETURN (const_int 0 [0])
>> (nil))))
>> (expr_list:DI (use (reg:DI 10 a0))
>> (nil)))
>> (barrier 439 438 617)
>>
>>
>> The edge 42 -> 64 already exists at the entry of vsetvl, so its not something
>> that the vsetvl/LCM creates.
>>
>> Entering Lazy VSETVL PASS
>> ...
>> ...
>> ;; 27 succs { 64 28 }
>> ;; 28 succs { 64 29 } <---
>> ;; 29 succs { 30 43 }
>> ;; 30 succs { 64 31 }
>> ;; 31 succs { 32 43 }
>> ;; 32 succs { 64 33 }
>> ;; 33 succs { 64 34 }
>> ;; 34 succs { 64 35 }
>> ;; 35 succs { 36 39 }
>> ;; 36 succs { 64 37 }
>> ;; 37 succs { 64 38 }
>> ;; 38 succs { 39 }
>> ;; 39 succs { 64 40 }
>> ;; 40 succs { 64 41 }
>> ;; 41 succs { 64 42 }
>> ;; 42 succs { 64 43 } <----
>> ...
>> ...
>>
>> In the working case, --param=vsetvl-strategy=simple, in cprop (just before
>> vsetvl), I don't see any jumps to the code label of bb 64 (with Unwinder
>> call)
>> either.
>>
>> I don't think vsetvl LCM is supposed to prune this edge out. In fact in the
>> routine to prepare LCM bitmaps I skip bb 42 (by checking for EDGE_CRITICAL in
>> invalid_opt_bb_p ()
>> The problem now happens in bb 28, which also happens to have edge to 64.
>>
>> So it seems to me that it should be safe to just skip the abnormal edge.
> But what state is in play that caused it to want to insert something?
> That's what needs to be understood here. I don't see anything in bb64
> that requires vsetvl to be in any particular state. So why did vsetvl
> insertion think that it needed to insert anything on the edge to bb64?
emit vsetvl (phase 4), sweeps thru all BBs and if they have any vsetvl info
(local or global) it visits all edges (including abnormal) and inserts the
vsetvl on edge.
It seems local would be when a BB needs vsetvl functionally (due to presence of
a V insn) vs. a global case being said BB part part of control flow eventually
leading to a BB with local vsetvl need.
So there is no real criteria for insertion here, its insert on every edge. Seems
like we need to skip some edges, somehow. Whether that needs to be unconditional
or say only if vsetvl info is non-local or excluding some bbs from LCM - I don't
know. Someone like Juzhe more familiar with the design would know better.
Thx,
-Vineet
