This is an archive of the discontinued LLVM Phabricator instance.

[NFC] [LLVM] Fix MaxFixPointIterations typo to match documentation
AbandonedPublic

Authored by gAlfonso-bit on Oct 20 2021, 10:53 AM.

Details

Summary

This should avoid documentation related warnings and make the code and documentation consistent

Diff Detail

Event Timeline

gAlfonso-bit created this revision.Oct 20 2021, 10:53 AM
gAlfonso-bit requested review of this revision.Oct 20 2021, 10:53 AM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
jdoerfert requested changes to this revision.Oct 20 2021, 11:18 AM

We do not want to talk about "fixed point" (as the alternative to "floating point") iterations but "fixpoint" iterations (in the sense of https://xavierleroy.org/courses/DSSS-2017/compiler.Fixpoint.html).

That said, the current Attributor docs are wrong once:

llvm/include/llvm/Transforms/IPO/Attributor.h
2007:  /// Maximum number of fixedpoint iterations.

and the code uses the wrong wording a few times:

llvm/lib/Transforms/IPO/Attributor.cpp
1331:  unsigned MaxFixedPointIterations;
1333:    MaxFixedPointIterations = MaxFixpointIterations.getValue();
1335:    MaxFixedPointIterations = SetFixpointIterations;
1415:  } while (!Worklist.empty() && (IterationCounter++ < MaxFixedPointIterations ||
1418:  if (IterationCounter > MaxFixedPointIterations && !Worklist.empty()) {
1421:                 << ore::NV("Iterations", MaxFixedPointIterations)
1425:    emitRemark<OptimizationRemarkMissed>(F, "FixedPoint", Remark);
1464:      IterationCounter != MaxFixedPointIterations) {
1466:           << IterationCounter << "/" << MaxFixedPointIterations

But this patch changes it in the wrong way.

This revision now requires changes to proceed.Oct 20 2021, 11:18 AM
gAlfonso-bit retitled this revision from [NFC] [LLVM] Fix MaxFixpointIterations typo to match documentation to [NFC] [LLVM] Fix MaxFixPointIterations typo to match documentation.

Addressed issues

gAlfonso-bit updated this revision to Diff 381055.
jdoerfert accepted this revision.Oct 20 2021, 5:31 PM

LG, thanks for the cleanup

This revision is now accepted and ready to land.Oct 20 2021, 5:31 PM

@jdoerfert Thank you, but I do not have commit permissions. Could you please commit this?

I need a name and email to attribute the patch to. Arc is not great doing this automagically.

This comment was removed by gAlfonso-bit.
This comment was removed by gAlfonso-bit.
This comment was removed by gAlfonso-bit.
gAlfonso-bit abandoned this revision.May 21 2022, 8:10 AM

Obsolete

Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2022, 8:10 AM