This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDag] Updated FoldConstantArithmetic method signature in preparation for merge with FoldConstantVectorArithmetic
ClosedPublic

Authored by justice_adams on Jan 16 2020, 1:12 PM.

Details

Summary

Updated FoldConstantArithmetic method signature to match that of FoldConstantVectorArithmetic in preparation for merging the two functions together

https://bugs.llvm.org/show_bug.cgi?id=36544

This is the first step in combining the various FoldConstantVectorArithmetic and FoldConstantVectorArithmetic functions into one FoldConstantArithmetic function.

I've ensured all lit tests pass on Windows and Linux with the following changes

Diff Detail

Event Timeline

justice_adams created this revision.Jan 16 2020, 1:12 PM

Thanks for cleaning this up!

The changes all look logically fine to me, but there are numerous formatting problems (80-cols, indentation, etc.).

Try running "clang-format" on the patch?

Thanks for cleaning this up!

The changes all look logically fine to me, but there are numerous formatting problems (80-cols, indentation, etc.).

Try running "clang-format" on the patch?

Depending on your workflow, this might be a useful script:
http://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting

Ran clang-format on the patch

@spatel My mistake! I have run clang-format on the patch and the diff should be cleaned up

RKSimon added inline comments.Jan 23 2020, 11:38 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
4883

(style) Remove braces

@RKSimon I removed the unnecessary braces. Good catch

spatel accepted this revision.Jan 24 2020, 4:37 AM

LGTM

This revision is now accepted and ready to land.Jan 24 2020, 4:37 AM

Thank you for the review @spatel . I do not have commit access, could you possibly commit this for me?

Thank you for the review @spatel . I do not have commit access, could you possibly commit this for me?

Sure - will push sometime in the next few hours.