This is an archive of the discontinued LLVM Phabricator instance.

[mips] [IAS] Make .module directives change AssemblerOptions->front().
ClosedPublic

Authored by tomatabacu on Jun 23 2015, 3:39 AM.

Diff Detail

Event Timeline

tomatabacu updated this revision to Diff 28215.Jun 23 2015, 3:39 AM
tomatabacu retitled this revision from to [mips] [IAS] Make .module directives change AssemblerOptions->front()..
tomatabacu updated this object.
tomatabacu edited the test plan for this revision. (Show Details)
tomatabacu added a reviewer: dsanders.
tomatabacu added subscribers: mpf, Unknown Object (MLST).

I was thinking of the following test:

# RUN: not llvm-mc %s -arch=mips -mcpu=mips32 -mattr=+fp64,-nooddspreg 2> %t0 | \
# RUN:   FileCheck %s

  .module nooddspreg
  add.s $f1, $f2, $f4
# CHECK: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

  .set oddspreg
  add.s $f1, $f2, $f4
# CHECK-NOT: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

  .set mips0
  add.s $f1, $f2, $f4
# CHECK: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

Currently, the last check should fail because AssemblerOptions->front() wasn't updated when we processed the ".module nooddspreg".

However, seeing as the IAS doesn't have support for .set oddspreg/nooddspreg, the only option left is to use .module/.set fp=23/64/xx, which I don't really know how to.
I would appreciate some help with this.

dsanders edited edge metadata.Jun 23 2015, 7:07 AM

I was thinking of the following test:

# RUN: not llvm-mc %s -arch=mips -mcpu=mips32 -mattr=+fp64,-nooddspreg 2> %t0 | \
# RUN:   FileCheck %s

  .module nooddspreg
  add.s $f1, $f2, $f4
# CHECK: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

  .set oddspreg
  add.s $f1, $f2, $f4
# CHECK-NOT: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

  .set mips0
  add.s $f1, $f2, $f4
# CHECK: :[[@LINE-1]]:15: error: -mno-odd-spreg prohibits the use of odd FPU registers

Currently, the last check should fail because AssemblerOptions->front() wasn't updated when we processed the ".module nooddspreg".

However, seeing as the IAS doesn't have support for .set oddspreg/nooddspreg, the only option left is to use .module/.set fp=23/64/xx, which I don't really know how to.
I would appreciate some help with this.

So the '.set mips0' currently reverts to the state before the '.module nooddspreg'. That sounds like a bug to me.

The suggested test case looks sensible to me and it should be easy to implement '.set nooddspreg'. Could you add the test case to the patch?

tomatabacu updated this revision to Diff 28254.Jun 23 2015, 9:57 AM
tomatabacu edited edge metadata.

Fixed a dumb bug in my original {set,clear}ModuleFeatureBits():
*The if was preventing the setting of the front options.
Added my proposed test case.
Added dependency on D10657.

dsanders accepted this revision.Jun 25 2015, 7:41 AM
dsanders edited edge metadata.

LGTM with a nit

test/MC/Mips/update-module-level-options.s
10 ↗(On Diff #28254)

It's probably best to be less strict on the column. This check will succeed if the error appeared for column 10 for example.

This revision is now accepted and ready to land.Jun 25 2015, 7:41 AM
This revision was automatically updated to reflect the committed changes.