[PATCH] D58170: [DTU] Refine the interface and logic of applyUpdates
Chijun Sima via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 20 10:20:54 PST 2019
NutshellySima added a comment.
In D58170#1404240 <https://reviews.llvm.org/D58170#1404240>, @brzycki wrote:
> @NutshellySima I like the idea of the new naming scheme that makes usage more explicit.
>
> I noticed there are no changes to `JumpThreading.cpp`, do they need to be altered in a separate patch or are you fine with all of `applyUpdates` implicitly becoming `applyUpdatesHinted` in `Lazy` mode?
>
> I also find the name `applyUpdatesHinted` a bit confusing and had to read your comments above to understand Hinted meant "we check the CFG for validity". Maybe use `applyUpdatesChecked` or `applyUpdatesValidated` as an alternative name that makes it a bit more clear that extra work must be done for this set of updates.
>From my perspective, it is nice to decouple two different updates submitting methods (accurate/inaccurate) from the way to flush these changes (Eager/Lazy).
But concerning most Lazy strategy users use `applyUpdatesChecked` and most Eager strategy users use `applyUpdatesAccurate`, I think it is OK to use `applyUpdates` as an alias of `applyUpdatesChecked` under Lazy strategy. :)
I'll rename `applyUpdatesHinted` as `applyUpdatesChecked` later.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58170/new/
https://reviews.llvm.org/D58170
More information about the llvm-commits
mailing list