[PATCH] D89917: Revert "SimplifyCFG: Clean up optforfuzzing implementation"

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 22 06:49:13 PDT 2020


lebedev.ri added a comment.

In D89917#2347088 <https://reviews.llvm.org/D89917#2347088>, @arsenm wrote:

> In D89917#2346421 <https://reviews.llvm.org/D89917#2346421>, @lebedev.ri wrote:
>
>> Thank you for the patch!
>>
>> In D89917#2346037 <https://reviews.llvm.org/D89917#2346037>, @arsenm wrote:
>>
>>> I disagree with this change. SimplifyCFG should continue to pass in options for transform options as is done for the others. I don't think avoiding duplication between the new PM and code that will be deleted is worth it
>>
>> I disagree here. Why is that beneficial, if we want to force-disable those transforms regardless of their previous configured status?
>
> Because SimplifyCFG is a standalone utility, not just a pass. It's a pass's responsibility to ensure attributes are respected, not individual transformations. If I'm writing another control flow pass (e.g. a structurizer) and calling these utilities, the transformation options should not be limited based on this extremely poorly specified attribute.

Let's back-track here.
Are you saying  those folds should not disable themselves in presence of `OptForFuzzing` attribute,
or that they should be guarded by their options?
Because agree with later, but disagree with former.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89917/new/

https://reviews.llvm.org/D89917



More information about the llvm-commits mailing list