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

Thu Oct 22 06:59:45 PDT 2020

arsenm added a comment.

In D89917#2347182 <https://reviews.llvm.org/D89917#2347182>, @lebedev.ri wrote:

> 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 that those folds should not disable themselves in presence of `OptForFuzzing` attribute,
> or that they should be guarded by their options?
> Because i agree with later, but disagree with former.

I'm saying these folds should be guarded by the SimplifyCFGOptions flags. The pass itself should initialize the option field based on the attribute. Other uses of SimplifyCFG the utilities can consider the attribute at their discretion

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89917