[PATCH] D62791: [CFLGraph] Add support for unary fneg instruction.
George Burgess IV via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 3 12:00:05 PDT 2019
george.burgess.iv added a comment.
Thanks you both! LGTM with a test, assuming that adding the test is straightforward.
Similar comments as on D62790 <https://reviews.llvm.org/D62790>: presuming that `fneg` doesn't operate on pointers, this code will just be a nop. If CFL* gets revived someday, we might end up trying to extract useful information from the flow of values through non-pointer-types, but as it stands, we just treat any `inttoptr` as magic and are conservative about it.
I suppose that if we wanted to be more explicit about this, we could have `add*Edge` return whether or not an edge was added, and just `assert(!add*Edge(...))` for all FP ops, but that's out of the scope of this patch IMO.
> it looks like test/Analysis/CFLAliasAnalysis is the right place
+1. CFLGraph bits are shared between both the Andersen/ and Steensgaard/ subdirectories, so it shouldn't make a difference which one we place the coverage line in.
> just proving that the new code is exercised with this change
For the reasons noted above, "it doesn't crash" is probably the best test we can have here at the moment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62791/new/
https://reviews.llvm.org/D62791
More information about the llvm-commits
mailing list