This is an archive of the discontinued LLVM Phabricator instance.

Fix misleading comment for getOrEnforceKnownAlignment
ClosedPublic

Authored by arsenm on Sep 9 2016, 6:12 PM.

Details

Reviewers
jlebar
efriedma
Summary

It does not return 0 to indicate failure, and returns the known alignment.

Diff Detail

Event Timeline

arsenm updated this revision to Diff 70938.Sep 9 2016, 6:12 PM
arsenm retitled this revision from to Fix misleading comment for getOrEnforceKnownAlignment.
arsenm updated this object.
arsenm added reviewers: jlebar, efriedma.
arsenm added a subscriber: llvm-commits.
jlebar added inline comments.Sep 9 2016, 6:15 PM
include/llvm/Transforms/Utils/Local.h
165

This prep phrase is unwieldy. Can we rephrase? (I'd give a suggestion but I'm not actually sure what it means.)

170

What do we return if the alignment can be modified?

arsenm updated this revision to Diff 70945.Sep 9 2016, 7:20 PM

Try to rephrase

jlebar accepted this revision.Sep 9 2016, 7:47 PM
jlebar edited edge metadata.

I'm happy with the edits below.

Thanks for fixing this.

include/llvm/Transforms/Utils/Local.h
166

I still can't tell what this function returns on "success". Perhaps s/increased/increased and PrefAlign returned/?

167

New paragraph before "It"?

This revision is now accepted and ready to land.Sep 9 2016, 7:47 PM
arsenm closed this revision.Sep 13 2016, 9:49 AM

r281350