[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