Hi Robin, On 4/8/25 21:56, Robin Dapp wrote: >>>>>>>> Yay ! It does work. Awesome. >>>>>>>> I've uploaded the further reduced test to PR/119533 >>>>>>> Hmm, I'm seeing the same ICE as before with my patch. Did you happen >>>>>>> to change >>>>>>> something else on your local tree still? >>>> Yeah I had some debug stuff lying around. In particular it is missing the >>>> initializer for kill bitmap. >>>> >>>> @@ -2698,6 +2713,7 @@ pre_vsetvl::compute_lcm_local_properties () >>>> m_avout = sbitmap_vector_alloc (last_basic_block_for_fn (cfun), >>>> num_exprs); >>>> >>>> bitmap_vector_clear (m_avloc, last_basic_block_for_fn (cfun)); >>>> + bitmap_vector_clear (m_kill, last_basic_block_for_fn (cfun)); >>>> bitmap_vector_clear (m_antloc, last_basic_block_for_fn (cfun)); >>>> bitmap_vector_ones (m_transp, last_basic_block_for_fn (cfun)); >>>> >>>> >>>>>> On top, I'm now seeing a ton of vsetvl test failures vs just the one I >>>>>> reported... No idea what I have been testing before. Grml. >>>> From your inline diff (with "!= 1" as cue) , I'd placed the check as >>>> follows >>>> >>>> if (!curr_info.valid_p () >>>> || eg->probability == profile_probability::never () >>>> || src_block_info.probability >>>> == profile_probability::uninitialized () >>>> /* When multiple set bits in earliest edge, such edge may >>>> have infinite loop in preds or succs or multiple conflict >>>> vsetvl expression which make such edge is unrelated. We >>>> don't perform fusion for such situation. */ >>>> || bitmap_count_bits (e) != 1) >>>> continue; >>>> >>>> + if (!bitmap_bit_p (m_transp[eg->src->index], expr_index)) >>>> + continue; >>>> >>>> With both of above changes both go tests passed in the morning. >>>> >>>>> Ah, of course the check was intended to be inside src_block_info.empty_p >>>>> () and >>>>> not outside. That gets rid of the test failures. But the go tests still >>>>> ICE >>>>> for me, one way or another. >>>> Now I moved the bitmap check further down inside src_block_info.empty_p () >>>> and >>>> both tests still pass. >>>> So looks like the kill bitmap init is essential as well. >>> Doh ! Something is weird, I did a clean everything and now I see the issues >>> still and it seems for baseline build, prinstine trunk is hitting ICE on >>> glibc >>> build as of >>> >>> 2025-04-08 0980a6ff7ae3 Daily bump. >>> >>> Let me untangle my testing first. >> The go issue, needs your transparent set check but we also need to tighten >> tighten invalid_opt_bb_p () for skipping successor edges being abnormal, >> currently it only does for predecessors only. If you recall this is >> something >> I'd tried last week, but that alone was not enough. >> >> So it seems we might have to seperate out the changes for PR/119547 (opencv) >> and >> 119533 (libgo) with latter dependent on former. >> >> On baseline Daily Bump of April 01, with attached patch including everything >> (just to test), we see 2 deltas >> >> rv64imafdcv_zvl256b_zba_zbb_zbs_zicond/ lp64d/ medlow | 303 / 101 | >> 16 >> / 4 | 67 / 12 | >> rv64imafdcv_zvl256b_zba_zbb_zbs_zicond/ lp64d/ medlow | 311 / 103 | >> 16 >> / 4 | 67 / 12 | >> >> FAIL: gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-10.c >> FAIL: gcc.target/riscv/rvv/vsetvl/avl_single-68.c >> >> I still need to figure out if vlmax_switch_vtype-10 is transparency related >> or >> the edge issue. > vlmax_switch_vtype-10 I already adjusted locally. It's related but the code > with the new changes is better than before. So IMHO no need to worry about > it.
Awesome, thx for the confirmation. I'll post a v2 for the go fix. -Vineet
