[PATCH] D118044: [ARM] Undeprecate complex IT blocks

Thu Jan 27 08:11:15 PST 2022

DavidSpickett added inline comments.

================
Comment at: llvm/docs/ReleaseNotes.rst:87
 * Added support for the Armv8.1-M PACBTI-M extension.
+* Removed the deprecation of ARMv8-A T32 Complex IT blocks. Such blocks
+  are now enabled by default, but may be turned off if required. No
----------------
I would mention the name of the option that changed default here. E.g.

"No deprecation warnings will be generated and `-mrestrict-it` is now always off by default. Previously it was on by default for Armv8 and off for all other architecture versions."

If I'm a user scanning release notes then I can grep my project makefiles and not have to care what Complex IT blocks even means if it's something I haven't specifically set.

================
Comment at: llvm/lib/Target/ARM/ARMSubtarget.cpp:62
    cl::values(clEnumValN(DefaultIT, "arm-default-it",
-                         "Generate IT block based on arch"),
+                         "Generate normal IT blocks"),
               clEnumValN(RestrictedIT, "arm-restrict-it",
----------------
This implies the two are the same:
DefaultIT: normal IT blocks
RestrictedIT: IT blocks minus complex IT blocks, so, normal IT blocks?

Like if someone said oh here's the box of normal puzzles I wouldn't expect to find a complex one in there.

Perhaps "Generate any type of IT block"? Then the second one is more obviously the first minus complex.

</nitpick>

================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:10962
     { // processInstruction() updates inITBlock state, we need to save it away
       bool wasInITBlock = inITBlock();

----------------
This is now unused.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118044