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

lgtm with a minor whitespace issue



================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1972
   }
-
   CI->setMetadata("heapallocsite", node);
----------------
Please revert the white space only change to this file.


================
Comment at: llvm/lib/CodeGen/MachineFunction.cpp:816
+
+  DIType *DI = dyn_cast<DIType>(MD);
+  CodeViewHeapAllocSites.push_back({BeginLabel, EndLabel, DI});
----------------
I guess it's reasonable to treat any non-DIType as "void". The only other 
reasonable thing to do would be to report an error, but it's not worth it.


================
Comment at: llvm/test/CodeGen/X86/label-heapallocsite.ll:1
+; RUN: llc -O0 < %s | FileCheck %s
+; FIXME: Add test for llc with optimizations once it is implemented.
----------------
akhuang wrote:
> hans wrote:
> > Does llc have a "-fast-isel" flag or similar that could be used instead, to 
> > make it more clear that it's fast-isel that's significant for the test?
> I couldn't find a flag that makes llc use fast-isel; it should soon work for 
> both cases though.
FWIW, -O0 is the typical way to enable fast isel in other codegen tests, so 
even if it's opaque, it's consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60800



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to