[PATCH] D128670: [SimplifyCFG] teach simplifycfg not to introduce ptrtoint for NI pointers

Wed Jul 6 12:02:39 PDT 2022

vtjnash added inline comments.

================
Comment at: llvm/test/Transforms/SimplifyCFG/nonintegral.ll:2
+; RUN: opt -simplifycfg -verify -S < %s | FileCheck %s
+; RUN: opt -passes=simplifycfg,verify -S < %s | FileCheck %s
+
----------------
nikic wrote:
> vtjnash wrote:
> > nikic wrote:
> > > Explicit verify is unnecessary. First RUN line can be dropped, they do the same thing. Please use update_test_checks.py.
> > I used update_test_checks.py to generate these tests, but disliked the results, and edited them to the form here. Has policy changed such that it is now required to be used verbatim?
> Yes, auto-generated check lines are preferred. If you want to make it more explicit what this test is about, maybe add a comment?
> 
> ```
> ; Check that ptrtoint is not produced for non-integral address spaces.
> ```
That seems to be at odds with both the testing guide (which implies it is optional) https://llvm.org/docs/TestingGuide.html#generating-assertions-in-regression-tests
and the tool itself (which states the results should be edited to remove extra lines, and the output is not designed to be representative of a good test):
https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/utils/update_test_checks.py#L27-L29

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128670