[PATCH] D99377: [PowerPC] Add ROP Protection to prologue and epilogue

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/ppc64-rop-protection.ll:135
+;
+; LE-P9-O0-LABEL: caller:
+; LE-P9-O0:       # %bb.0: # %entry
----------------
amyk wrote:
> Since the test file is large, I'm wondering if some of the CHECKs can be combined/maybe we can use some common `check-prefixes` (if that's a good idea)?
> For example, it looks like `LE-P9-O0` and `LE-P8-O0` look similar. As well as `BE-P9` and `BE-P8`.
> Since the test file is large, I'm wondering if some of the CHECKs can be combined/maybe we can use some common `check-prefixes` (if that's a good idea)?
> For example, it looks like `LE-P9-O0` and `LE-P8-O0` look similar. As well as `BE-P9` and `BE-P8`.

I agree that this test is really large (~3500 lines).

When I wrote the original testcase I didn't use `update_llc_test_checks.py` and I had merged a number of these tests. I was using CHECK, CHECK-NEXT, CHECK-DAG, and I was trying to get rid of lines of code that didn't matter. After that, I added the `-O0` tests and realized that this was all becoming too hard to maintain. Therefore, I switched to using the script to generate the checks. I could certainly merge some of these tests but I fear that would make things hard to maintain again. If two codegen patterns are the same now there is no guarantee that two months from now that will be the case and whoever is updating the test will have a hard time figuring out how to update it. 

As a result I would prefer to leave things as they are now.
The other option is to have only the most basic checks that would work in all situations. 
```
CHECK-LABEL:
CHECK: hashst <OFFSET>
CHECK: hashchk <OFFSET>
CHECK: blr
```
I would be happy to (significantly!) shirk the test like that but I am afraid that it might not cover enough code...
Anyway, let me know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99377