This is an archive of the discontinued LLVM Phabricator instance.

[Headers][doc] Add "add/sub/mul" intrinsic descriptions to avx2intrin.h
ClosedPublic

Authored by probinson on May 8 2023, 6:37 AM.

Diff Detail

Event Timeline

probinson created this revision.May 8 2023, 6:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2023, 6:37 AM
probinson requested review of this revision.May 8 2023, 6:37 AM
craig.topper added inline comments.May 15 2023, 5:12 PM
clang/lib/Headers/avx2intrin.h
156

Why do some return descriptions include the type like [4 x i64] but some don't?

1346

I think minuend and subtrahend are swapped here.

pengfei added inline comments.May 15 2023, 10:31 PM
clang/lib/Headers/avx2intrin.h
1339

It better to move it to \code{.operation} for consistency. Same for the below.

probinson added inline comments.May 23 2023, 10:17 AM
clang/lib/Headers/avx2intrin.h
156

My policy has been to provide the type for element sizes other than byte. So, I haven't been saying [32 x i8] but I do say [4 x i64] or whatever.
Although, I have also tended to say "integer vector" when it's a byte vector, and I'll make that consistent here as well.

1339

Okay.

1346

Thanks for catching that!

Address review comments

craig.topper added inline comments.May 23 2023, 11:23 AM
clang/lib/Headers/avx2intrin.h
456

Intel intrinsics guide says

dst[31:0] := a[31:0] - a[63:32]
dst[63:32] := a[95:64] - a[127:96]
dst[95:64] := b[31:0] - b[63:32]
dst[127:96] := b[95:64] - b[127:96]
dst[159:128] := a[159:128] - a[191:160]
dst[191:160] := a[223:192] - a[255:224]
dst[223:192] := b[159:128] - b[191:160]
dst[255:224] := b[223:192] - b[255:224]
dst[MAX:256] := 0

So I think the operands are in the wrong order here?

488

Operands are reversed?

probinson updated this revision to Diff 525632.May 25 2023, 8:28 AM

Correct order of horizontal operands

clang/lib/Headers/avx2intrin.h
456

Words fail me. Also diagrams. I wanted the add and sub descriptions to look similar, and copy-pasted from add to sub without verifying the order.

pengfei accepted this revision.May 25 2023, 6:55 PM

LGTM except for a possible typo.

clang/lib/Headers/avx2intrin.h
412

underflow?

448

typo or intended?

448

underflow.

This revision is now accepted and ready to land.May 25 2023, 6:55 PM
probinson marked an inline comment as done.May 30 2023, 10:15 AM
probinson added inline comments.
clang/lib/Headers/avx2intrin.h
412

I don't often see "underflow" applied to integer operations. Technically, any signed add or subtract could either overflow or underflow, depending on the sign and magnitude of the operands. I think just saying "overflow" is clear enough?

448

The i31 is a typo, fixed.

This revision was landed with ongoing or failed builds.May 30 2023, 10:15 AM
This revision was automatically updated to reflect the committed changes.
probinson marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2023, 10:15 AM