[PATCH] MachineFunction is exposed to X86AsmParser.

Yuri Gorshenin ygorshenin at chromium.org
Wed Apr 2 11:14:14 PDT 2014


  Investigating why compilation of llvm-c-test fails.


================
Comment at: lib/Target/X86/AsmParser/X86AsmInstrumentation.h:32
@@ +31,3 @@
+private:
+  X86AsmInstrumentation(const X86AsmInstrumentation &) LLVM_DELETED_FUNCTION;
+  void operator=(const X86AsmInstrumentation &)LLVM_DELETED_FUNCTION;
----------------
Evgeniy Stepanov wrote:
> This is an unrelated change, right? It should be a separate CL.
> If you want, I could commit this chunk right now.
> 
Deleted, thanks. This class don't have an internal state now, so it's ok to omit void copy ctor and copy operator.

================
Comment at: include/llvm/MC/MCTargetOptions.h:18
@@ +17,3 @@
+  None,   // No instrumentation at all.
+  Address // Instrument instructions with memory arguments.
+};
----------------
Evgeniy Stepanov wrote:
> SanitizeAddress
> 
> I still think that bitfields work better here, even though there may be incompatible instrumentation modes.
> 
Done.

================
Comment at: include/llvm/MC/MCTargetOptions.h:24
@@ +23,3 @@
+public:
+  /// AsmInstrumentationMode - This flag is sed by the
+  /// asm-instrumentation=mode option is specified on the command
----------------
Evgeniy Stepanov wrote:
> s/sed/set/
> I'd rather explain what this flag does, not how it gets set. Like "enables AddressSanitizer instrumentation at machine level".
Done.

================
Comment at: include/llvm/MC/MCTargetOptions.h:31
@@ +30,3 @@
+
+  MCTargetOptions();
+  void InitFromFlags();
----------------
Evgeniy Stepanov wrote:
> Where is the definition of this constructor?
> 
> What happens to MCTargetOptions without the corresponding clang patch. They should default to no instrumentation.
Sorry, I forget to upload a source file. Fixed.

================
Comment at: include/llvm/Target/TargetMachine.h:111
@@ -110,2 +110,3 @@
   void resetTargetOptions(const MachineFunction *MF) const;
+  const TargetOptions &getTargetOptions() const { return Options; }
 
----------------
Evgeniy Stepanov wrote:
> Why? Options is a public member.
Done.


http://llvm-reviews.chandlerc.com/D3106



More information about the llvm-commits mailing list