[PATCH] D16807: [mips] MUL macro variations

[Daniel Sanders via llvm-commits]

dsanders added inline comments.

================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:3691
@@ +3690,3 @@
+    if (AssemblerOptions.back()->isReorder())
+      createNop(false, IDLoc, Instructions);
+    emitII(Mips::BREAK,6, 0, IDLoc, Instructions);
----------------
obucina wrote:
> It is unclear to me what do you suggest regarding micromips potential problem with branch offset. Please be more precise.
The offset needs to be different for microMIPS so you need to test for microMIPS and select between the MIPS and microMIPS value.
The way to fix this depends on whether the offset for the microMIPS case is a constant or is variable.

If the offset for the microMIPS case is a constant then we can use magic numbers for the moment (but please document them with comments). If the offset for the microMIPS case is variable for any reason then we'll have to find some way to account for the variability somehow (which could require the work explained at the bottom of this comment).

Ideally, we'd avoid any magic numbers using symbols and symbol references but it's not possible to do that right now (see below)

> For reference, my overall plan for solving this kind of problem is to finish rewriting the emit*() functions such that they directly write to the
> target streamer. This allows us to drop labels wherever we need them using the target streamer methods.

The reason we can't use symbols to avoid magic numbers at the moment is because this migration work is not yet complete. The problem is that we collect the expansion in a list of instructions and we can't put symbol definitions in this list. To eliminate the magic numbers we need to finish this work and use the target streamers as a stream. This will allow us to drop temporary symbols wherever we need them.


http://reviews.llvm.org/D16807