[PATCH] D62532: [AIX] Implement function descriptor on SDAG

Tue Jun 4 14:39:34 PDT 2019

Xiangling_L marked 2 inline comments as done.
Xiangling_L added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp:1878
   case Instruction::Call:
+    // On AIX, call lowering should always use the DAG-ISEL path. The callee of
+    // the call instruction has to be mapped to the symbol for the functions
----------------
jasonliu wrote:
> hubert.reinterpretcast wrote:
> > Is this statement of fundamental (permanent) fact? Please make the comment clear on that point. Clarify the relation between the first and the second sentences. For example, does the second sentence indicate something that is only done if we return `false` for AIX?
> I vaguely remember the discussion being... we will disable this fast-isel path for now on AIX and get the non-fast-isel path working first. We would deal with fast-isel path later down the road. 
> If my memory is correct, then I don't think it is a "permanent fact". So maybe we don't want to use words like "should always", and state that this would be "for now" and we will evaluate the situation later. 
If we don't always ban fast-isel of call loweing on AIX, does that mean we need to duplicate logic of creating MCSymbolSDNode/MCSymbolXCOFF for function entry point on fast-isel path?

================
Comment at: llvm/lib/Target/PowerPC/PPC.h:92
     /// for PIC calls on Linux and ELF systems.
+
     MO_PLT = 1,
----------------
jasonliu wrote:
> Extra blank line?
Thanks. will address this in the next update.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62532