[PATCH] D38530: [AArch64] Add support for dllimport of values and functions

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd accepted this revision.
compnerd added a comment.
This revision is now accepted and ready to land.

Seems fine with the adjustments.



================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:3714
       Subtarget->ClassifyGlobalReference(GV, getTargetMachine());
+  EVT PtrVT = getPointerTy(DAG.getDataLayout());
+  SDValue Result;
----------------
Can you lower this to ~L3725.


================
Comment at: lib/Target/AArch64/AArch64MCInstLower.cpp:41
+  const Triple &TheTriple = Printer.TM.getTargetTriple();
+  if (TheTriple.isOSBinFormatCOFF()) {
+    assert(TheTriple.isOSWindows() &&
----------------
Early return please:

    if (!TheTriple.isOSBinFormatCOFF())
      return Printer.getSymbol(MO.getGlobal());
    ...


================
Comment at: test/CodeGen/AArch64/dllimport.ll:1
+; RUN: llc -mtriple aarch64-windows -filetype asm -o - %s | FileCheck %s
+
----------------
I prefer the canonicalized triple `aarch64-unknown-windows-msvc`


================
Comment at: test/CodeGen/AArch64/dllimport.ll:6
+declare dllimport arm_aapcs_vfpcc i32 @external()
+declare arm_aapcs_vfpcc i32 @internal()
+
----------------
Are the `arm_aapcs_vfpcc` correct here?  The variant of AAPCS64 isn't marked differently I thought.  The same through out.


https://reviews.llvm.org/D38530