[PATCH] D91734: [FastISel] Flush local value map on every instruction

Paul Robinson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 6 08:19:58 PST 2021


probinson added a comment.

In D91734#2480930 <https://reviews.llvm.org/D91734#2480930>, @dblaikie wrote:

> I haven't looked closely, but I take it this one test modification seems reasonable to you? I guess we're stepping into some subexpressions on a function call that we previously didn't? (they didn't have any instructions attributed to them until now, but it's not unreasonable that they have them attributed - for materializing constants to pass to a function call when the function call/constants are split over multiple lines, etc)

Exactly.  In each case, the test is trying to "next" over a function call; gcc attributes all parameter evaluation to the function name, while clang will attribute each parameter to its own location.  And when the parameters span multiple source lines, the is_stmt heuristic kicks in, so we stop on each line with actual parameters.

This is not ideal IMO; I'd rather be more principled about (a) stop at every parameter, or (b) stop at no parameters.  But without reworking how we do is_stmt, I think fiddling the test to do enough single-steps to actually get past the function call is okay.


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

https://reviews.llvm.org/D91734



More information about the llvm-commits mailing list