[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