[PATCH] Forbid the use of registers t6, t7 and t8 if the target is MIPS NaCl.

Mark Seaborn mseaborn at chromium.org
Wed Feb 5 12:20:59 PST 2014


  LGTM with the test change below


================
Comment at: lib/Target/Mips/MipsRegisterInfo.cpp:137
@@ -136,1 +136,3 @@
 
+  // Reserved for NaCl use.  T6 contains the mask for sandboxing control flow
+  // (indirect jumps and calls).  T7 contains the mask for sandboxing memory
----------------
Just a suggestion, but this would be more readable next to the code, one per line, something like this:

  Reserved.set(Mips::T6); // Reserved for control flow mask
  Reserved.set(Mips::T7); // Reserved for memory access mask
  Reserved.set(Mips::T8); // Reserved for thread pointer

================
Comment at: test/CodeGen/Mips/nacl-reserved-regs.ll:71
@@ +70,3 @@
+  %27 = load i32* @g27
+  call void @f2(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %5, i32 %6, i32 %7,
+                i32 %8, i32 %9, i32 %10, i32 %11, i32 %12, i32 %13, i32 %14,
----------------
It still seems to me that this wouldn't necessarily be a robust test.  Most of these args are passed on the stack, so the args could be copied from @gN to the stack one by one, using only a single temp register.

I see what you mean about my suggestion not working, because t6-t8 are temp registers, not callee-saved.

I think you'd need to avoid doing a function call.  How about using volatile memory accesses, like this?

@var = external global i32

define void @test() {
  %val1 = load volatile i32* @var
  %val2 = load volatile i32* @var
  %val3 = load volatile i32* @var
  %val4 = load volatile i32* @var
  %val5 = load volatile i32* @var
  %val6 = load volatile i32* @var
  %val7 = load volatile i32* @var
  %val8 = load volatile i32* @var
  %val9 = load volatile i32* @var
  %val10 = load volatile i32* @var
  %val11 = load volatile i32* @var
  %val12 = load volatile i32* @var
  %val13 = load volatile i32* @var
  %val14 = load volatile i32* @var
  %val15 = load volatile i32* @var
  %val16 = load volatile i32* @var
  store volatile i32 %val1, i32* @var
  store volatile i32 %val2, i32* @var
  store volatile i32 %val3, i32* @var
  store volatile i32 %val4, i32* @var
  store volatile i32 %val5, i32* @var
  store volatile i32 %val6, i32* @var
  store volatile i32 %val7, i32* @var
  store volatile i32 %val8, i32* @var
  store volatile i32 %val9, i32* @var
  store volatile i32 %val10, i32* @var
  store volatile i32 %val11, i32* @var
  store volatile i32 %val12, i32* @var
  store volatile i32 %val13, i32* @var
  store volatile i32 %val14, i32* @var
  store volatile i32 %val15, i32* @var
  store volatile i32 %val16, i32* @var
  ret void
}



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



More information about the llvm-commits mailing list