{"action":"created","issue":{"url":"https://api.github.com/repos/golang/go/issues/35881","repository_url":"https://api.github.com/repos/golang/go","labels_url":"https://api.github.com/repos/golang/go/issues/35881/labels{/name}","comments_url":"https://api.github.com/repos/golang/go/issues/35881/comments","events_url":"https://api.github.com/repos/golang/go/issues/35881/events","html_url":"https://github.com/golang/go/issues/35881","id":529585752,"node_id":"MDU6SXNzdWU1Mjk1ODU3NTI=","number":35881,"title":"cmd/internal/obj/x86: pad jumps to avoid Intel erratum","user":{"login":"rsc","id":104030,"node_id":"MDQ6VXNlcjEwNDAzMA==","avatar_url":"https://avatars1.githubusercontent.com/u/104030?v=4","gravatar_id":"","url":"https://api.github.com/users/rsc","html_url":"https://github.com/rsc","followers_url":"https://api.github.com/users/rsc/followers","following_url":"https://api.github.com/users/rsc/following{/other_user}","gists_url":"https://api.github.com/users/rsc/gists{/gist_id}","starred_url":"https://api.github.com/users/rsc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rsc/subscriptions","organizations_url":"https://api.github.com/users/rsc/orgs","repos_url":"https://api.github.com/users/rsc/repos","events_url":"https://api.github.com/users/rsc/events{/privacy}","received_events_url":"https://api.github.com/users/rsc/received_events","type":"User","site_admin":false},"labels":[{"id":373401956,"node_id":"MDU6TGFiZWwzNzM0MDE5NTY=","url":"https://api.github.com/repos/golang/go/labels/NeedsDecision","name":"NeedsDecision","color":"ededed","default":false,"description":"Feedback is required from experts, contributors, and/or the community before a change can be made."}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/golang/go/milestones/95","html_url":"https://github.com/golang/go/milestone/95","labels_url":"https://api.github.com/repos/golang/go/milestones/95/labels","id":4185562,"node_id":"MDk6TWlsZXN0b25lNDE4NTU2Mg==","number":95,"title":"Go1.14","description":"","creator":{"login":"andybons","id":365204,"node_id":"MDQ6VXNlcjM2NTIwNA==","avatar_url":"https://avatars3.githubusercontent.com/u/365204?v=4","gravatar_id":"","url":"https://api.github.com/users/andybons","html_url":"https://github.com/andybons","followers_url":"https://api.github.com/users/andybons/followers","following_url":"https://api.github.com/users/andybons/following{/other_user}","gists_url":"https://api.github.com/users/andybons/gists{/gist_id}","starred_url":"https://api.github.com/users/andybons/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andybons/subscriptions","organizations_url":"https://api.github.com/users/andybons/orgs","repos_url":"https://api.github.com/users/andybons/repos","events_url":"https://api.github.com/users/andybons/events{/privacy}","received_events_url":"https://api.github.com/users/andybons/received_events","type":"User","site_admin":false},"open_issues":81,"closed_issues":473,"state":"open","created_at":"2019-03-30T21:11:32Z","updated_at":"2019-12-02T22:43:08Z","due_on":"2020-02-01T08:00:00Z","closed_at":null},"comments":9,"created_at":"2019-11-27T21:44:21Z","updated_at":"2019-12-03T10:58:12Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"Intel [erratum SKX102](https://www.intel.com/content/www/us/en/support/articles/000055650/processors.html) “Processor May Behave Unpredictably Under Complex Sequence of Conditions Which Involve Branches That Cross 64-Byte Boundaries” applies to:\n\n - Intel® Celeron® Processor 4000 Series\n - Intel® Celeron® Processor G Series 10th Generation \n - Intel® Core™ i5 Processors 10th Generation \n - Intel® Core™ i7 Processors 6th Generation \n - Intel® Core™ i3 Processors 6th Generation \n - Intel® Core™ i5 Processors 6th Generation \n - Intel® Core™ i7 Processors 6th Generation \n - Intel® Core™ m Processors 7th Generation \n - Intel® Core™ i3 Processors 7th Generation \n - Intel® Core™ i5 Processors 7th Generation \n - Intel® Core™ i7 Processors 7th Generation \n - Intel® Core™ m Processors 8th Generation \n - Intel® Core™ i3 Processors 8th Generation \n - Intel® Core™ i5 Processors 8th Generation \n - Intel® Core™ i7 Processors 8th Generation \n - Intel® Core™ m Processors 9th Generation \n - Intel® Core™ i9 Processors\n - Intel® Core™ X-series Processors\n - Intel® Pentium® Gold Processor Series\n - Intel® Pentium® Processor G Series\n - Intel® Xeon® Processor E3 v5 Family\n - Intel® Xeon® Processor E3 v6 Family 2nd Generation \n - Intel® Xeon® Scalable Processors\n - Intel® Xeon® E Processor\n - Intel® Xeon® Scalable Processors\n - Intel® Xeon® W Processor\n\nThere is a microcode fix that can be applied by the BIOS to avoid the incorrect execution. It stops any jump (jump, jcc, call, ret, direct, indirect) from being cached in the decoded icache when the instruction ends at or crosses a 32-byte boundary. [Intel says](https://www.intel.com/content/dam/support/us/en/documents/processors/mitigations-jump-conditional-code-erratum.pdf#page=7):\n\n> Intel has observed performance effects associated with the workaround [microcode fix] ranging from 0-4% on many industry-standard benchmarks. In subcomponents of these benchmarks, Intel has observed outliers higher than the 0-4% range. Other workloads not observed by Intel may behave differently.\n\nThe suggested workaround for the workaround is to insert padding so that fused branch sequences never end at or cross a 64-byte boundary. This means the whole CMP+Jcc, not just Jcc. \n\n[CL 206837](https://go-review.googlesource.com/c/go/+/206837/) adds a new environment variable to set the padding policy. The original CL used $GO_X86_PADJUMP but the discussion has moved on to using $GOAMD64, which would avoid breaking the build cache.\n\nThere are really two questions here:\n\n - What is the right amount of padding to insert by default?\n - Given that default, what additional control do developers need over the padding?\n\nIn general, we try to do the right thing for developers so that they don't have to keep track of every last CPU erratum. That seems like it would suggest we should do the padding automatically. Otherwise Go programs on this very large list of processors have the possibility of behaving “unpredictably.\" \n\nIf the overheads involved are small enough and we are 100% confident in the padding code, we could stop there and just leave it on unconditionally. It seems like that's what we should do rather than open the door to arbitrary compiler option configuration in $GOAMD64, and all the complexity that comes with it.\n\nSo what are the overheads? Here is an estimate.\n\n```\n$ ls -l $(which go)\n-rwxr-xr-x  1 rsc  primarygroup  15056484 Nov 15 15:10 /Users/rsc/go/bin/go\n$ go tool objdump $(which go) >go.dump\n$ grep -c '^TEXT' go.dump\n10362\n$ cat go.dump | awk '$2~/^0x/ {print $4, length($3)/2}' | sort | uniq -c | egrep 'CALL| J|RET' >jumps\n$ cat jumps | awk '{n=$3; if($2 ~ /^J[^M]/) n += 3; total += $1*(n/16)*(n+1)/2} END{print total}'\n251848\n$ \n```\n\nThis go command has 10,362 functions, and the padding required for instructions crossing or ending at a 16-byte boundary should average out to 251,848 extra bytes. The awk adds 3 to conditional jumps to simulate fusing of a preceding register-register CMP instruction. \n\nChanging function alignment to 32 bytes would halve the padding added (saving 125,924 bytes) but add 16 more bytes on average to each of the functions (adding 165,792 bytes). So changing function alignment does not seem to be worthwhile.\n\nSame for a smaller binary:\n\n```\n$ ls -l $(which gofmt)\n-rwxr-xr-x  1 rsc  primarygroup  3499584 Nov 15 15:09 /Users/rsc/go/bin/gofmt\n$ go tool objdump $(which gofmt) >gofmt.dump\n$ grep -c '^TEXT' gofmt.dump\n2956\n$ cat gofmt.dump | awk '$2~/^0x/ {print $4, length($3)/2}' | sort | uniq -c | egrep 'CALL| J|RET' >jumps\n$ cat jumps | awk '{n=$3; if($2 ~ /^J[^M]/) n += 3; total += $1*(n/16)*(n+1)/2} END{print total}'\n58636.8\n$ \n```\n\nChanging alignment to 32 would save 29,318.4 bytes but add 47,296 bytes.\n\nOverall, the added bytes are 1.67% in both the go command and gofmt. This is not nothing, but it seems like a small price to pay for correct execution, and if it makes things faster on some systems, even better.\n\nMy tentative conclusion then would be that we should just turn this on by default and not have an option. Thoughts?"},"comment":{"url":"https://api.github.com/repos/golang/go/issues/comments/561115479","html_url":"https://github.com/golang/go/issues/35881#issuecomment-561115479","issue_url":"https://api.github.com/repos/golang/go/issues/35881","id":561115479,"node_id":"MDEyOklzc3VlQ29tbWVudDU2MTExNTQ3OQ==","user":{"login":"markdryan","id":984073,"node_id":"MDQ6VXNlcjk4NDA3Mw==","avatar_url":"https://avatars3.githubusercontent.com/u/984073?v=4","gravatar_id":"","url":"https://api.github.com/users/markdryan","html_url":"https://github.com/markdryan","followers_url":"https://api.github.com/users/markdryan/followers","following_url":"https://api.github.com/users/markdryan/following{/other_user}","gists_url":"https://api.github.com/users/markdryan/gists{/gist_id}","starred_url":"https://api.github.com/users/markdryan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/markdryan/subscriptions","organizations_url":"https://api.github.com/users/markdryan/orgs","repos_url":"https://api.github.com/users/markdryan/repos","events_url":"https://api.github.com/users/markdryan/events{/privacy}","received_events_url":"https://api.github.com/users/markdryan/received_events","type":"User","site_admin":false},"created_at":"2019-12-03T10:58:12Z","updated_at":"2019-12-03T10:58:12Z","author_association":"CONTRIBUTOR","body":"I've updated the patch in gerrit to improve the prefix counting code and to add a unit test for the prefix counting code."}}
{"actor":{"display_login":"markdryan"}}