Hi all
Saleem, in 2014 you added the .thumb_set test case. I’ve found a difference in
behaviour of .thumb_set compared to .set. I was hoping to resolve this
difference as it will allow me to reapply r239440.
The issue i’m seeing is that we define an alpha function, then beta function,
then assign alpha to beta. But given that beta has already been defined, this
would mean we are redefining it at the point of the .thumb_set:
.text
.thumb
.thumb_set alias_defined_data, bedazzle
.type alpha,%function
alpha:
nop
.type beta,%function
beta:
bkpt
.thumb_set beta, alpha
The above code currently passes. However, if I change .thumb_set to .set then I
get
../test/MC/ARM/thumb_set.s:60:13: error: redefinition of 'beta'
.set beta, alpha
I would like to make .thumb_set throw the same error here as I believe that is
how it should behave in this case.
Please let me know if you are ok with this, or some other approach. I’m
unfamiliar with it so its possible that what i’ve found isn’t a bug at all but
just how we want it to behave.
Cheers,
Pete
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150609/a67f487e/attachment.html>
Hey Salem, Any chance you’ve had time to take a look at this? If you prefer, I can prepare a patch with the change i’d like to make and we can see if you are happy with it. Cheers, Pete> On Jun 9, 2015, at 4:09 PM, Pete Cooper <peter_cooper at apple.com> wrote: > > Hi all > > Saleem, in 2014 you added the .thumb_set test case. I’ve found a difference in behaviour of .thumb_set compared to .set. I was hoping to resolve this difference as it will allow me to reapply r239440. > > The issue i’m seeing is that we define an alpha function, then beta function, then assign alpha to beta. But given that beta has already been defined, this would mean we are redefining it at the point of the .thumb_set: > > .text > .thumb > > .thumb_set alias_defined_data, bedazzle > > .type alpha,%function > alpha: > nop > > .type beta,%function > beta: > bkpt > > .thumb_set beta, alpha > > The above code currently passes. However, if I change .thumb_set to .set then I get > > ../test/MC/ARM/thumb_set.s:60:13: error: redefinition of 'beta' > .set beta, alpha > > I would like to make .thumb_set throw the same error here as I believe that is how it should behave in this case. > > Please let me know if you are ok with this, or some other approach. I’m unfamiliar with it so its possible that what i’ve found isn’t a bug at all but just how we want it to behave. > > Cheers, > Pete-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150612/5fef0b0e/attachment.html>
For what it is worth it, gas has this strange behaviour. I agree it is strange, but why is it an issue for r239440? Will I get a failure if I revert it? Cheers, Rafael On 12 June 2015 at 12:17, Pete Cooper <peter_cooper at apple.com> wrote:> Hey Salem, > > Any chance you’ve had time to take a look at this? > > If you prefer, I can prepare a patch with the change i’d like to make and we > can see if you are happy with it. > > Cheers, > Pete > > On Jun 9, 2015, at 4:09 PM, Pete Cooper <peter_cooper at apple.com> wrote: > > Hi all > > Saleem, in 2014 you added the .thumb_set test case. I’ve found a difference > in behaviour of .thumb_set compared to .set. I was hoping to resolve this > difference as it will allow me to reapply r239440. > > The issue i’m seeing is that we define an alpha function, then beta > function, then assign alpha to beta. But given that beta has already been > defined, this would mean we are redefining it at the point of the > .thumb_set: > > .text > .thumb > > .thumb_set alias_defined_data, bedazzle > > .type alpha,%function > alpha: > nop > > .type beta,%function > beta: > bkpt > > .thumb_set beta, alpha > > The above code currently passes. However, if I change .thumb_set to .set > then I get > > ../test/MC/ARM/thumb_set.s:60:13: error: redefinition of 'beta' > .set beta, alpha > > I would like to make .thumb_set throw the same error here as I believe that > is how it should behave in this case. > > Please let me know if you are ok with this, or some other approach. I’m > unfamiliar with it so its possible that what i’ve found isn’t a bug at all > but just how we want it to behave. > > Cheers, > Pete > >
On Tue, Jun 9, 2015 at 4:09 PM, Pete Cooper <peter_cooper at apple.com> wrote:> Hi all > > Saleem, in 2014 you added the .thumb_set test case. I’ve found a > difference in behaviour of .thumb_set compared to .set. I was hoping to > resolve this difference as it will allow me to reapply r239440. > > The issue i’m seeing is that we define an alpha function, then beta > function, then assign alpha to beta. But given that beta has already been > defined, this would mean we are redefining it at the point of the > .thumb_set: > > .text > .thumb > > .thumb_set alias_defined_data, bedazzle > > .type alpha,%function > alpha: > nop > > .type beta,%function > beta: > bkpt > > .thumb_set beta, alpha > > The above code currently passes. However, if I change .thumb_set to .set > then I get > > *../test/MC/ARM/thumb_set.s:60:13: **error: **redefinition of 'beta'* > .set beta, alpha > > I would like to make .thumb_set throw the same error here as I believe > that is how it should behave in this case. > > Please let me know if you are ok with this, or some other approach. I’m > unfamiliar with it so its possible that what i’ve found isn’t a bug at all > but just how we want it to behave. >The behavior was taken from GAS. I don't know if there are applications using this and dependent on the behavior. Given that this is a GNU extension, I think its better to keep the behavior similar to GAS rather than implement and diverge.> Cheers, > Pete >-- Saleem Abdulrasool compnerd (at) compnerd (dot) org -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150612/a63dd4b3/attachment.html>
On 13 June 2015 at 04:12, Saleem Abdulrasool <compnerd at compnerd.org> wrote:> The behavior was taken from GAS. I don't know if there are applications > using this and dependent on the behavior. Given that this is a GNU > extension, I think its better to keep the behavior similar to GAS rather > than implement and diverge.I kind of agree with Saleem, here. We don't want to deviate, even from silly behaviour, when implementing a GNU extension. If this brings problems, we may add a warning, but not much more, I'd say. cheers, --renato