Thanks Jeff and Robin for comments, sorry for late reply. > Conceptually the rounding mode is just a property. The call, in effect, > should demand a "normal" rounding mode and set the rounding mode to > unknown if I understand how this is supposed to work. If my > understanding is wrong, then maybe that's where we should start -- with > a good description of the problem ;-)
I think we are on the same page of how it works, I may need to take a look at how x86 taking care of this. > That's probably dead code at this point. IIRC rth did further work in > this space because inserting in the end of the block with the abnormal > edge isn't semantically correct. > It's been 20+ years, but IIRC he adjusted the PRE bitmaps so that we > never would need to do an insertion on an abnormal edge. Search for > EDGE_ABNORMAL in gcse.cc. That is quite old up to a point, will have a try for the EDGE_ABNORMAL case. > Having said that, it looks like Pan's patch just tries to move some of > the dirty work from the backend to the mode-switching pass by making it > easier to do something after a call. I believe I asked for that back in > one of the reviews even? Yes, that is what I would like to do in this PATCH, as the following up of some comments from Robin in previous. Pan -----Original Message----- From: Robin Dapp <rdapp....@gmail.com> Sent: Monday, October 2, 2023 4:26 PM To: Jeff Law <jeffreya...@gmail.com>; Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org Cc: rdapp....@gmail.com; juzhe.zh...@rivai.ai; Wang, Yanzhang <yanzhang.w...@intel.com>; kito.ch...@gmail.com Subject: Re: [PATCH v1] Mode-Switching: Add optional EMIT_AFTER hook > Conceptually the rounding mode is just a property. The call, in > effect, should demand a "normal" rounding mode and set the rounding > mode to unknown if I understand how this is supposed to work. If my > understanding is wrong, then maybe that's where we should start -- > with a good description of the problem ;-) That's also what I what struggled with last time this was discussed. Normally, mode switching is used to switch to a requested mode for an insn or a call and potentially switch back afterwards. For those riscv intrinsics that specify a variable, non-default rounding mode we have two options: - Save and restore before and after each mode-changing intrinsic fegetround old_rounding fesetround new_rounding actual instruction fesetround old_rounding) - Have mode switching do it for us (lazily) to avoid most of the storing of the old rounding mode by storing an (e.g.) function-level rounding-mode backup value. The backup value is used to lazily restore the currently valid rounding mode. The problem with this now is that whenever fesetround gets called our backup is outdated. Therefore we need to update our backup after each function call (as fesetround can of course be present anywhere) and this is where most of the complications come from. So in that case the callee _does_ impact the caller via the backup clobbering. That was one of my complaints about the whole procedure last time. Besides, I didn't see the need for those intrinsics anyway and would much rather have explicit fesetround calls but well :) Having said that, it looks like Pan's patch just tries to move some of the dirty work from the backend to the mode-switching pass by making it easier to do something after a call. I believe I asked for that back in one of the reviews even? Regards Robin