[PATCH] D73601: [X86] Fix isOffsetSuitableForCodeModel

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 28 18:49:57 PST 2020


craig.topper added a comment.

Instead of a new parameter is it possible to work something into the AddressingMode struct so that it gets restored when we restore from a backup AM?

Do you have a test case?



================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:213
+    bool foldOffsetIntoAddress(uint64_t Offset, X86ISelAddressMode &AM,
+                               bool ForceSymbolicDisp = false);
     bool matchLoadInAddress(LoadSDNode *N, X86ISelAddressMode &AM);
----------------
This name doesn't match what's used in the definition of the function.


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:1594
+  if (!matchAddressRecursively(Handle.getValue().getOperand(1), AM, Depth + 1,
+                               HasSymbolicDisp) &&
+      !matchAddressRecursively(Handle.getValue().getOperand(0), AM, Depth + 1,
----------------
Do we need to resync HasSymbolicDisp from the restored AM if one of the matchAddressRecursively calls failed the match but had modified the flag?


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:2098
     X86ISelAddressMode Backup = AM;
-    if (matchAddressRecursively(N.getOperand(0), AM, Depth+1)) {
+    bool ForceSymbolicDisp = false;
+    if (matchAddressRecursively(N.getOperand(0), AM, Depth + 1,
----------------
Why is it called Force here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73601





More information about the llvm-commits mailing list