[PATCH] D66329: [PowerPC] [Peephole] fold frame offset by using index form to save add.

Stefan Pintilie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 13 15:02:30 PDT 2019


stefanp added a comment.

I do have a couple additional comments.



================
Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.cpp:2703
+  // TODO: sync the logic between instrHasImmForm() and ImmToIdxMap.
+  if (!HasImmForm)
+    return false;
----------------
Since `HasImmForm` is only used here you can put it directly into the if statement.

However, I'm a little confused as to why you have to call `instrHasImmForm` and then check the return. It seems like you start with the immediate form and then move on to get the X-Form for it and then you ask to see if there is an immediate. Is it to fill in the `III` object with the correct flags?

If that is the case we need to have another function to set the flags correctly for an arbitrary immediate opcode. That, however is out of the scope of this patch.


================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.h:68
+    return ImmToIdxMap;
+  }
+
----------------
Instead of returning the map like this I would prefer to have a couple of functions along the lines of `hasIndexFormforImm` and `getIndexFormFormImm`. You don't need to use my function names but I think you get the idea. The register info owns that map and should be able to answer questions about it.


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

https://reviews.llvm.org/D66329





More information about the llvm-commits mailing list