This is an archive of the discontinued LLVM Phabricator instance.

[Thumb] Add some integer abs testcases for different typesizes.
ClosedPublic

Authored by ikulagin on Sep 15 2018, 12:04 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

ikulagin created this revision.Sep 15 2018, 12:04 PM
ikulagin added inline comments.Sep 15 2018, 12:06 PM
test/CodeGen/Thumb/iabs.ll
18 ↗(On Diff #165656)

Why do we need to check in such a manner without taking into account specific instructions but only counting their number?

RKSimon resigned from this revision.Sep 29 2018, 3:00 AM
RKSimon added a subscriber: RKSimon.Feb 4 2019, 3:45 AM

What's happening with this? I'm keen to get D49837 in soon

test/CodeGen/Thumb/iabs.ll
3 ↗(On Diff #165656)

Instead of the counts, just running llc and checking the full codegen would be a lot cleaner.

I don't think the update_llc_test_checks script supports thumb triples but that'd be even easier.....

Herald added a project: Restricted Project. · View Herald TranscriptFeb 4 2019, 3:45 AM

What's happening with this? I'm keen to get D49837 in soon

This changes are already exist in D49837

What's happening with this? I'm keen to get D49837 in soon

This changes are already exist in D49837

It'd be better to get this committed against current trunk codegen so D49837 shows the codegen change

@t.p.northover You blocked this change - any comments?

RKSimon added inline comments.Feb 28 2019, 1:09 AM
test/CodeGen/Thumb/iabs.ll
20 ↗(On Diff #188677)

This test is just a copy of test_i32 - lets keep test_i32 and get rid of this.

3 ↗(On Diff #165656)

Drop the llvm-objdump run - we can get everything we need (full instruction codegen) from the llc - that checks we have the right #instructions.

From what I can tell, t.p.northover was added as a blocking reviewer, he didn't block this patch.

I agree with RKSimon, you can remove the odd "count" check and just use the update script.

test/CodeGen/Thumb/iabs.ll
3 ↗(On Diff #165656)

Yep, I agree. Maybe change the triple to thumbv6m-none-eabi too? Then just run update_llc_test_checks.py on it.

ikulagin updated this revision to Diff 188785.Feb 28 2019, 1:50 PM

Update by update_llc_test_checks.py.

RKSimon accepted this revision.Feb 28 2019, 1:59 PM

LGTM - do you have commit access yet?

No I don't. Could you commit it, please?

ikulagin removed a reviewer: t.p.northover. ikulagin added 1 blocking reviewer(s): RKSimon.Feb 28 2019, 2:07 PM
RKSimon accepted this revision.Mar 1 2019, 3:21 AM

LGTM

This revision is now accepted and ready to land.Mar 1 2019, 3:21 AM
This revision was automatically updated to reflect the committed changes.