This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Refactor visitIntrinsicCall so it doesn't return a const char*
ClosedPublic

Authored by gchatelet on Apr 30 2019, 6:18 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

gchatelet created this revision.Apr 30 2019, 6:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2019, 6:18 AM
gchatelet updated this revision to Diff 197309.Apr 30 2019, 6:20 AM
  • Fix typo
Harbormaster completed remote builds in B31152: Diff 197309.
courbet added inline comments.Apr 30 2019, 6:32 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
5494 ↗(On Diff #197309)

Please update the comment.

7542 ↗(On Diff #197309)

So this is not really an NFC as this is is no longer executed on setjmp, longjmp, and clear_cache. Here ends my knowledge, let's find reviewers who can comment on whether this is OK or not :)

gchatelet updated this revision to Diff 197314.Apr 30 2019, 6:38 AM
gchatelet marked an inline comment as done.
  • Update documentation
sanjoy added a subscriber: reames.May 4 2019, 12:32 AM
sanjoy added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
6511 ↗(On Diff #197314)

Needs clang-format

Also is this ever null?

7542 ↗(On Diff #197309)

This change in behavior seems fine to me but CC @reames to be sure.

gchatelet updated this revision to Diff 198262.May 6 2019, 6:35 AM
gchatelet marked 2 inline comments as done.
  • clang-format
gchatelet marked an inline comment as done.
gchatelet added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
6511 ↗(On Diff #197314)
7542 ↗(On Diff #197309)

Thx for the review @sanjoy . @reames do you mind having a look?

gchatelet marked an inline comment as done.May 13 2019, 4:57 AM
gchatelet added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7542 ↗(On Diff #197309)

A gentle ping @reames

I'll submit this one shortly. @reames please let me know if you have any concerns.

This revision was not accepted when it landed; it landed in state Needs Review.May 20 2019, 1:49 AM
This revision was automatically updated to reflect the committed changes.